netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] bridge: update sysfs link names if port device names have changed
@ 2010-05-06 18:32 Simon Arlott
  2010-05-06 18:53 ` Stephen Hemminger
  2010-05-06 19:01 ` [PATCH (v2)] " Simon Arlott
  0 siblings, 2 replies; 7+ messages in thread
From: Simon Arlott @ 2010-05-06 18:32 UTC (permalink / raw)
  To: shemminger; +Cc: netdev

Links for each port are created in sysfs using the device
name, but this could be changed after being added to the
bridge.

As well as being unable to remove interfaces after this
occurs (because userspace tools don't recognise the new
name, and the kernel won't recognise the old name), adding
another interface with the old name to the bridge will
cause an error trying to create the sysfs link.

This fixes the problem by listening for NETDEV_CHANGENAME
notifications and renaming the link.

https://bugzilla.kernel.org/show_bug.cgi?id=12743

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
 fs/sysfs/symlink.c       |    1 +
 net/bridge/br_if.c       |    2 +-
 net/bridge/br_notify.c   |    4 ++++
 net/bridge/br_private.h  |    5 +++++
 net/bridge/br_sysfs_if.c |   19 +++++++++++++++++--
 5 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index b93ec51..942f239 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -261,3 +261,4 @@ const struct inode_operations sysfs_symlink_inode_operations = {
 
 EXPORT_SYMBOL_GPL(sysfs_create_link);
 EXPORT_SYMBOL_GPL(sysfs_remove_link);
+EXPORT_SYMBOL_GPL(sysfs_rename_link);
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 0b6b1f2..17175a5 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -132,7 +132,7 @@ static void del_nbp(struct net_bridge_port *p)
 	struct net_bridge *br = p->br;
 	struct net_device *dev = p->dev;
 
-	sysfs_remove_link(br->ifobj, dev->name);
+	sysfs_remove_link(br->ifobj, p->sysfs_name);
 
 	dev_set_promiscuity(dev, -1);
 
diff --git a/net/bridge/br_notify.c b/net/bridge/br_notify.c
index 763a3ec..9004406 100644
--- a/net/bridge/br_notify.c
+++ b/net/bridge/br_notify.c
@@ -82,6 +82,10 @@ static int br_device_event(struct notifier_block *unused, unsigned long event, v
 	case NETDEV_UNREGISTER:
 		br_del_if(br, dev);
 		break;
+
+	case NETDEV_CHANGENAME:
+		br_sysfs_renameif(p);
+		break;
 	}
 
 	/* Events that may cause spanning tree to refresh */
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 846d7d1..dcbe744 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -96,6 +96,9 @@ struct net_bridge_port
 {
 	struct net_bridge		*br;
 	struct net_device		*dev;
+#ifdef CONFIG_SYSFS
+	char				sysfs_name[IFNAMSIZ];
+#endif
 	struct list_head		list;
 
 	/* STP */
@@ -433,6 +436,7 @@ extern void br_ifinfo_notify(int event, struct net_bridge_port *port);
 /* br_sysfs_if.c */
 extern const struct sysfs_ops brport_sysfs_ops;
 extern int br_sysfs_addif(struct net_bridge_port *p);
+extern void br_sysfs_renameif(struct net_bridge_port *p);
 
 /* br_sysfs_br.c */
 extern int br_sysfs_addbr(struct net_device *dev);
@@ -441,6 +445,7 @@ extern void br_sysfs_delbr(struct net_device *dev);
 #else
 
 #define br_sysfs_addif(p)	(0)
+#define br_sysfs_renameif(p)	do { } while(0)
 #define br_sysfs_addbr(dev)	(0)
 #define br_sysfs_delbr(dev)	do { } while(0)
 #endif /* CONFIG_SYSFS */
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index 0b99164..6702d7d 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -246,7 +246,7 @@ const struct sysfs_ops brport_sysfs_ops = {
 /*
  * Add sysfs entries to ethernet device added to a bridge.
  * Creates a brport subdirectory with bridge attributes.
- * Puts symlink in bridge's brport subdirectory
+ * Puts symlink in bridge's brif subdirectory
  */
 int br_sysfs_addif(struct net_bridge_port *p)
 {
@@ -265,7 +265,22 @@ int br_sysfs_addif(struct net_bridge_port *p)
 			goto out2;
 	}
 
-	err = sysfs_create_link(br->ifobj, &p->kobj, p->dev->name);
+	strncpy(p->sysfs_name, p->dev->name, IFNAMSIZ);
+	err = sysfs_create_link(br->ifobj, &p->kobj, p->sysfs_name);
 out2:
 	return err;
 }
+
+/* Rename bridge's brif symlink */
+void br_sysfs_renameif(struct net_bridge_port *p)
+{
+	struct net_bridge *br = p->br;
+	int err;
+
+	err = sysfs_rename_link(br->ifobj, &p->kobj,
+		p->sysfs_name, p->dev->name);
+	if (err)
+		printk(KERN_ERR "%s: unable to rename sysfs link %s to %s (%d)",
+			br->dev->name, p->sysfs_name, p->dev->name, err);
+	strncpy(p->sysfs_name, p->dev->name, IFNAMSIZ);
+}
-- 
1.7.0.4

-- 
Simon Arlott

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

* Re: [PATCH] bridge: update sysfs link names if port device names have changed
  2010-05-06 18:32 [PATCH] bridge: update sysfs link names if port device names have changed Simon Arlott
@ 2010-05-06 18:53 ` Stephen Hemminger
  2010-05-06 19:01 ` [PATCH (v2)] " Simon Arlott
  1 sibling, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2010-05-06 18:53 UTC (permalink / raw)
  To: Simon Arlott; +Cc: netdev

On Thu, 06 May 2010 19:32:29 +0100
Simon Arlott <simon@fire.lp0.eu> wrote:

> Links for each port are created in sysfs using the device
> name, but this could be changed after being added to the
> bridge.
> 
> As well as being unable to remove interfaces after this
> occurs (because userspace tools don't recognise the new
> name, and the kernel won't recognise the old name), adding
> another interface with the old name to the bridge will
> cause an error trying to create the sysfs link.
> 
> This fixes the problem by listening for NETDEV_CHANGENAME
> notifications and renaming the link.
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=12743
> 
> Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
> ---
>  fs/sysfs/symlink.c       |    1 +
>  net/bridge/br_if.c       |    2 +-
>  net/bridge/br_notify.c   |    4 ++++
>  net/bridge/br_private.h  |    5 +++++
>  net/bridge/br_sysfs_if.c |   19 +++++++++++++++++--
>  5 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
> index b93ec51..942f239 100644
> --- a/fs/sysfs/symlink.c
> +++ b/fs/sysfs/symlink.c
> @@ -261,3 +261,4 @@ const struct inode_operations sysfs_symlink_inode_operations = {
>  
>  EXPORT_SYMBOL_GPL(sysfs_create_link);
>  EXPORT_SYMBOL_GPL(sysfs_remove_link);
> +EXPORT_SYMBOL_GPL(sysfs_rename_link);
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index 0b6b1f2..17175a5 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -132,7 +132,7 @@ static void del_nbp(struct net_bridge_port *p)
>  	struct net_bridge *br = p->br;
>  	struct net_device *dev = p->dev;
>  
> -	sysfs_remove_link(br->ifobj, dev->name);
> +	sysfs_remove_link(br->ifobj, p->sysfs_name);
>  
>  	dev_set_promiscuity(dev, -1);
>  
> diff --git a/net/bridge/br_notify.c b/net/bridge/br_notify.c
> index 763a3ec..9004406 100644
> --- a/net/bridge/br_notify.c
> +++ b/net/bridge/br_notify.c
> @@ -82,6 +82,10 @@ static int br_device_event(struct notifier_block *unused, unsigned long event, v
>  	case NETDEV_UNREGISTER:
>  		br_del_if(br, dev);
>  		break;
> +
> +	case NETDEV_CHANGENAME:
> +		br_sysfs_renameif(p);
> +		break;
>  	}
>  
>  	/* Events that may cause spanning tree to refresh */
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 846d7d1..dcbe744 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -96,6 +96,9 @@ struct net_bridge_port
>  {
>  	struct net_bridge		*br;
>  	struct net_device		*dev;
> +#ifdef CONFIG_SYSFS
> +	char				sysfs_name[IFNAMSIZ];
> +#endif
>  	struct list_head		list;

Ok, but the sysfs_name should go at end of net_bridge_port structure
since it is only used in special case code. And putting in middle
of structure kills cache locality.

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

* [PATCH (v2)] bridge: update sysfs link names if port device names have changed
  2010-05-06 18:32 [PATCH] bridge: update sysfs link names if port device names have changed Simon Arlott
  2010-05-06 18:53 ` Stephen Hemminger
@ 2010-05-06 19:01 ` Simon Arlott
  2010-05-06 19:11   ` Stephen Hemminger
  1 sibling, 1 reply; 7+ messages in thread
From: Simon Arlott @ 2010-05-06 19:01 UTC (permalink / raw)
  To: shemminger; +Cc: netdev

Links for each port are created in sysfs using the device
name, but this could be changed after being added to the
bridge.

As well as being unable to remove interfaces after this
occurs (because userspace tools don't recognise the new
name, and the kernel won't recognise the old name), adding
another interface with the old name to the bridge will
cause an error trying to create the sysfs link.

This fixes the problem by listening for NETDEV_CHANGENAME
notifications and renaming the link.

https://bugzilla.kernel.org/show_bug.cgi?id=12743

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
Updated to force rename rollback if sysfs_rename_link fails.

 fs/sysfs/symlink.c       |    1 +
 net/bridge/br_if.c       |    2 +-
 net/bridge/br_notify.c   |    7 +++++++
 net/bridge/br_private.h  |    5 +++++
 net/bridge/br_sysfs_if.c |   29 +++++++++++++++++++++++++++--
 5 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index b93ec51..942f239 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -261,3 +261,4 @@ const struct inode_operations sysfs_symlink_inode_operations = {
 
 EXPORT_SYMBOL_GPL(sysfs_create_link);
 EXPORT_SYMBOL_GPL(sysfs_remove_link);
+EXPORT_SYMBOL_GPL(sysfs_rename_link);
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 0b6b1f2..17175a5 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -132,7 +132,7 @@ static void del_nbp(struct net_bridge_port *p)
 	struct net_bridge *br = p->br;
 	struct net_device *dev = p->dev;
 
-	sysfs_remove_link(br->ifobj, dev->name);
+	sysfs_remove_link(br->ifobj, p->sysfs_name);
 
 	dev_set_promiscuity(dev, -1);
 
diff --git a/net/bridge/br_notify.c b/net/bridge/br_notify.c
index 763a3ec..576a1b0 100644
--- a/net/bridge/br_notify.c
+++ b/net/bridge/br_notify.c
@@ -34,6 +34,7 @@ static int br_device_event(struct notifier_block *unused, unsigned long event, v
 	struct net_device *dev = ptr;
 	struct net_bridge_port *p = dev->br_port;
 	struct net_bridge *br;
+	int err;
 
 	/* not a port of a bridge */
 	if (p == NULL)
@@ -82,6 +83,12 @@ static int br_device_event(struct notifier_block *unused, unsigned long event, v
 	case NETDEV_UNREGISTER:
 		br_del_if(br, dev);
 		break;
+
+	case NETDEV_CHANGENAME:
+		err = br_sysfs_renameif(p);
+		if (err)
+			return NOTIFY_BAD;
+		break;
 	}
 
 	/* Events that may cause spanning tree to refresh */
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 846d7d1..4309bd8 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -96,6 +96,9 @@ struct net_bridge_port
 {
 	struct net_bridge		*br;
 	struct net_device		*dev;
+#ifdef CONFIG_SYSFS
+	char				sysfs_name[IFNAMSIZ];
+#endif
 	struct list_head		list;
 
 	/* STP */
@@ -433,6 +436,7 @@ extern void br_ifinfo_notify(int event, struct net_bridge_port *port);
 /* br_sysfs_if.c */
 extern const struct sysfs_ops brport_sysfs_ops;
 extern int br_sysfs_addif(struct net_bridge_port *p);
+extern int br_sysfs_renameif(struct net_bridge_port *p);
 
 /* br_sysfs_br.c */
 extern int br_sysfs_addbr(struct net_device *dev);
@@ -441,6 +445,7 @@ extern void br_sysfs_delbr(struct net_device *dev);
 #else
 
 #define br_sysfs_addif(p)	(0)
+#define br_sysfs_renameif(p)	(0)
 #define br_sysfs_addbr(dev)	(0)
 #define br_sysfs_delbr(dev)	do { } while(0)
 #endif /* CONFIG_SYSFS */
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index 0b99164..a7997a7 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -246,7 +246,7 @@ const struct sysfs_ops brport_sysfs_ops = {
 /*
  * Add sysfs entries to ethernet device added to a bridge.
  * Creates a brport subdirectory with bridge attributes.
- * Puts symlink in bridge's brport subdirectory
+ * Puts symlink in bridge's brif subdirectory
  */
 int br_sysfs_addif(struct net_bridge_port *p)
 {
@@ -265,7 +265,32 @@ int br_sysfs_addif(struct net_bridge_port *p)
 			goto out2;
 	}
 
-	err = sysfs_create_link(br->ifobj, &p->kobj, p->dev->name);
+	strncpy(p->sysfs_name, p->dev->name, IFNAMSIZ);
+	err = sysfs_create_link(br->ifobj, &p->kobj, p->sysfs_name);
 out2:
 	return err;
 }
+
+/* Rename bridge's brif symlink */
+int br_sysfs_renameif(struct net_bridge_port *p)
+{
+	struct net_bridge *br = p->br;
+	int err;
+
+	/* If a rename fails, the rollback will cause another
+	 * rename call with the existing name.
+	 */
+	if (!strncmp(p->sysfs_name, p->dev->name, IFNAMSIZ))
+		return 0;
+
+	err = sysfs_rename_link(br->ifobj, &p->kobj,
+		p->sysfs_name, p->dev->name);
+	if (err) {
+		printk(KERN_ERR "%s: unable to rename sysfs link %s to %s (%d)",
+			br->dev->name, p->sysfs_name, p->dev->name, err);
+	} else {
+		strncpy(p->sysfs_name, p->dev->name, IFNAMSIZ);
+	}
+
+	return err;
+}
-- 
1.7.0.4

-- 
Simon Arlott

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

* Re: [PATCH (v2)] bridge: update sysfs link names if port device names have changed
  2010-05-06 19:01 ` [PATCH (v2)] " Simon Arlott
@ 2010-05-06 19:11   ` Stephen Hemminger
  2010-05-06 19:37     ` [PATCH (v4)] " Simon Arlott
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2010-05-06 19:11 UTC (permalink / raw)
  To: Simon Arlott; +Cc: netdev

On Thu, 06 May 2010 20:01:44 +0100
Simon Arlott <simon@fire.lp0.eu> wrote:

> +
> +	case NETDEV_CHANGENAME:
> +		err = br_sysfs_renameif(p);
> +		if (err)
> +			return NOTIFY_BAD;
> +		break;

I think you want:
 if (err)
	return notifier_from_errno(err);


+	err = sysfs_rename_link(br->ifobj, &p->kobj,
+		p->sysfs_name, p->dev->name);
+	if (err) {
+		printk(KERN_ERR "%s: unable to rename sysfs link %s to %s (%d)",
+			br->dev->name, p->sysfs_name, p->dev->name, err);

This should not be KERN_ERR but KERN_NOTICE, and use new wrapper macros.

	if (err)
		netdev_notice(br->dev, "unable to rename sysfs link %s to %s".
			 p->sysfs_name, p->dev->name, err)

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

* [PATCH (v4)] bridge: update sysfs link names if port device names have changed
  2010-05-06 19:11   ` Stephen Hemminger
@ 2010-05-06 19:37     ` Simon Arlott
  2010-05-06 19:40       ` Stephen Hemminger
  2010-05-06 19:50       ` [PATCH (v4+)] " Stephen Hemminger
  0 siblings, 2 replies; 7+ messages in thread
From: Simon Arlott @ 2010-05-06 19:37 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

Links for each port are created in sysfs using the device
name, but this could be changed after being added to the
bridge.

As well as being unable to remove interfaces after this
occurs (because userspace tools don't recognise the new
name, and the kernel won't recognise the old name), adding
another interface with the old name to the bridge will
cause an error trying to create the sysfs link.

This fixes the problem by listening for NETDEV_CHANGENAME
notifications and renaming the link.

https://bugzilla.kernel.org/show_bug.cgi?id=12743

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
Updated notifier return value and error message logging.
Changed to use strlcpy instead of strncpy.

 fs/sysfs/symlink.c       |    1 +
 net/bridge/br_if.c       |    2 +-
 net/bridge/br_notify.c   |    7 +++++++
 net/bridge/br_private.h  |    6 ++++++
 net/bridge/br_sysfs_if.c |   29 +++++++++++++++++++++++++++--
 5 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index b93ec51..942f239 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -261,3 +261,4 @@ const struct inode_operations sysfs_symlink_inode_operations = {
 
 EXPORT_SYMBOL_GPL(sysfs_create_link);
 EXPORT_SYMBOL_GPL(sysfs_remove_link);
+EXPORT_SYMBOL_GPL(sysfs_rename_link);
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 0b6b1f2..17175a5 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -132,7 +132,7 @@ static void del_nbp(struct net_bridge_port *p)
 	struct net_bridge *br = p->br;
 	struct net_device *dev = p->dev;
 
-	sysfs_remove_link(br->ifobj, dev->name);
+	sysfs_remove_link(br->ifobj, p->sysfs_name);
 
 	dev_set_promiscuity(dev, -1);
 
diff --git a/net/bridge/br_notify.c b/net/bridge/br_notify.c
index 763a3ec..c3b0c80 100644
--- a/net/bridge/br_notify.c
+++ b/net/bridge/br_notify.c
@@ -34,6 +34,7 @@ static int br_device_event(struct notifier_block *unused, unsigned long event, v
 	struct net_device *dev = ptr;
 	struct net_bridge_port *p = dev->br_port;
 	struct net_bridge *br;
+	int err;
 
 	/* not a port of a bridge */
 	if (p == NULL)
@@ -82,6 +83,12 @@ static int br_device_event(struct notifier_block *unused, unsigned long event, v
 	case NETDEV_UNREGISTER:
 		br_del_if(br, dev);
 		break;
+
+	case NETDEV_CHANGENAME:
+		err = br_sysfs_renameif(p);
+		if (err)
+			return notifier_from_errno(err);
+		break;
 	}
 
 	/* Events that may cause spanning tree to refresh */
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 846d7d1..58f1dd7 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -128,6 +128,10 @@ struct net_bridge_port
 	struct hlist_head		mglist;
 	struct hlist_node		rlist;
 #endif
+
+#ifdef CONFIG_SYSFS
+	char				sysfs_name[IFNAMSIZ];
+#endif
 };
 
 struct net_bridge
@@ -433,6 +437,7 @@ extern void br_ifinfo_notify(int event, struct net_bridge_port *port);
 /* br_sysfs_if.c */
 extern const struct sysfs_ops brport_sysfs_ops;
 extern int br_sysfs_addif(struct net_bridge_port *p);
+extern int br_sysfs_renameif(struct net_bridge_port *p);
 
 /* br_sysfs_br.c */
 extern int br_sysfs_addbr(struct net_device *dev);
@@ -441,6 +446,7 @@ extern void br_sysfs_delbr(struct net_device *dev);
 #else
 
 #define br_sysfs_addif(p)	(0)
+#define br_sysfs_renameif(p)	(0)
 #define br_sysfs_addbr(dev)	(0)
 #define br_sysfs_delbr(dev)	do { } while(0)
 #endif /* CONFIG_SYSFS */
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index 0b99164..51b6b52 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -246,7 +246,7 @@ const struct sysfs_ops brport_sysfs_ops = {
 /*
  * Add sysfs entries to ethernet device added to a bridge.
  * Creates a brport subdirectory with bridge attributes.
- * Puts symlink in bridge's brport subdirectory
+ * Puts symlink in bridge's brif subdirectory
  */
 int br_sysfs_addif(struct net_bridge_port *p)
 {
@@ -265,7 +265,32 @@ int br_sysfs_addif(struct net_bridge_port *p)
 			goto out2;
 	}
 
-	err = sysfs_create_link(br->ifobj, &p->kobj, p->dev->name);
+	strlcpy(p->sysfs_name, p->dev->name, IFNAMSIZ);
+	err = sysfs_create_link(br->ifobj, &p->kobj, p->sysfs_name);
 out2:
 	return err;
 }
+
+/* Rename bridge's brif symlink */
+int br_sysfs_renameif(struct net_bridge_port *p)
+{
+	struct net_bridge *br = p->br;
+	int err;
+
+	/* If a rename fails, the rollback will cause another
+	 * rename call with the existing name.
+	 */
+	if (!strncmp(p->sysfs_name, p->dev->name, IFNAMSIZ))
+		return 0;
+
+	err = sysfs_rename_link(br->ifobj, &p->kobj,
+		p->sysfs_name, p->dev->name);
+	if (err) {
+		netdev_notice(br->dev, "unable to rename sysfs link %s to %s (%d)",
+			p->sysfs_name, p->dev->name, err);
+	} else {
+		strlcpy(p->sysfs_name, p->dev->name, IFNAMSIZ);
+	}
+
+	return err;
+}
-- 
1.7.0.4

-- 
Simon Arlott

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

* Re: [PATCH (v4)] bridge: update sysfs link names if port device names have changed
  2010-05-06 19:37     ` [PATCH (v4)] " Simon Arlott
@ 2010-05-06 19:40       ` Stephen Hemminger
  2010-05-06 19:50       ` [PATCH (v4+)] " Stephen Hemminger
  1 sibling, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2010-05-06 19:40 UTC (permalink / raw)
  To: Simon Arlott; +Cc: netdev

On Thu, 06 May 2010 20:37:11 +0100
Simon Arlott <simon@fire.lp0.eu> wrote:

> Links for each port are created in sysfs using the device
> name, but this could be changed after being added to the
> bridge.
> 
> As well as being unable to remove interfaces after this
> occurs (because userspace tools don't recognise the new
> name, and the kernel won't recognise the old name), adding
> another interface with the old name to the bridge will
> cause an error trying to create the sysfs link.
> 
> This fixes the problem by listening for NETDEV_CHANGENAME
> notifications and renaming the link.
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=12743
> 
> Signed-off-by: Simon Arlott <simon@fire.lp0.eu>

Looks good
Acked-by: Stephen Hemminger <shemminger@vyatta.com>


-- 

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

* [PATCH (v4+)] bridge: update sysfs link names if port device names have changed
  2010-05-06 19:37     ` [PATCH (v4)] " Simon Arlott
  2010-05-06 19:40       ` Stephen Hemminger
@ 2010-05-06 19:50       ` Stephen Hemminger
  1 sibling, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2010-05-06 19:50 UTC (permalink / raw)
  To: David Miller; +Cc: Simon Arlott, netdev


From: Simon Arlott <simon@fire.lp0.eu>

Links for each port are created in sysfs using the device
name, but this could be changed after being added to the
bridge.

As well as being unable to remove interfaces after this
occurs (because userspace tools don't recognise the new
name, and the kernel won't recognise the old name), adding
another interface with the old name to the bridge will
cause an error trying to create the sysfs link.

This fixes the problem by listening for NETDEV_CHANGENAME
notifications and renaming the link.

https://bugzilla.kernel.org/show_bug.cgi?id=12743

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>

---
Modified to apply to net-next and fix checkpatch warnings -- Stephen

 fs/sysfs/symlink.c       |    1 +
 net/bridge/br_if.c       |    2 +-
 net/bridge/br_notify.c   |    7 +++++++
 net/bridge/br_private.h  |    6 ++++++
 net/bridge/br_sysfs_if.c |   32 +++++++++++++++++++++++++++-----
 5 files changed, 42 insertions(+), 6 deletions(-)

--- a/fs/sysfs/symlink.c	2010-04-20 08:22:12.000000000 -0700
+++ b/fs/sysfs/symlink.c	2010-05-06 12:41:36.488153157 -0700
@@ -261,3 +261,4 @@ const struct inode_operations sysfs_syml
 
 EXPORT_SYMBOL_GPL(sysfs_create_link);
 EXPORT_SYMBOL_GPL(sysfs_remove_link);
+EXPORT_SYMBOL_GPL(sysfs_rename_link);
--- a/net/bridge/br_if.c	2010-05-06 12:24:59.000000000 -0700
+++ b/net/bridge/br_if.c	2010-05-06 12:41:36.488153157 -0700
@@ -133,7 +133,7 @@ static void del_nbp(struct net_bridge_po
 	struct net_bridge *br = p->br;
 	struct net_device *dev = p->dev;
 
-	sysfs_remove_link(br->ifobj, dev->name);
+	sysfs_remove_link(br->ifobj, p->sysfs_name);
 
 	dev_set_promiscuity(dev, -1);
 
--- a/net/bridge/br_notify.c	2010-05-06 12:24:30.000000000 -0700
+++ b/net/bridge/br_notify.c	2010-05-06 12:43:27.776801235 -0700
@@ -34,6 +34,7 @@ static int br_device_event(struct notifi
 	struct net_device *dev = ptr;
 	struct net_bridge_port *p = dev->br_port;
 	struct net_bridge *br;
+	int err;
 
 	/* not a port of a bridge */
 	if (p == NULL)
@@ -83,6 +84,12 @@ static int br_device_event(struct notifi
 		br_del_if(br, dev);
 		break;
 
+	case NETDEV_CHANGENAME:
+		err = br_sysfs_renameif(p);
+		if (err)
+			return notifier_from_errno(err);
+		break;
+
 	case NETDEV_PRE_TYPE_CHANGE:
 		/* Forbid underlaying device to change its type. */
 		return NOTIFY_BAD;
--- a/net/bridge/br_private.h	2010-05-06 08:43:18.000000000 -0700
+++ b/net/bridge/br_private.h	2010-05-06 12:41:36.488153157 -0700
@@ -139,6 +139,10 @@ struct net_bridge_port
 	struct hlist_head		mglist;
 	struct hlist_node		rlist;
 #endif
+
+#ifdef CONFIG_SYSFS
+	char				sysfs_name[IFNAMSIZ];
+#endif
 };
 
 struct br_cpu_netstats {
@@ -455,6 +459,7 @@ extern void br_ifinfo_notify(int event, 
 /* br_sysfs_if.c */
 extern const struct sysfs_ops brport_sysfs_ops;
 extern int br_sysfs_addif(struct net_bridge_port *p);
+extern int br_sysfs_renameif(struct net_bridge_port *p);
 
 /* br_sysfs_br.c */
 extern int br_sysfs_addbr(struct net_device *dev);
@@ -463,6 +468,7 @@ extern void br_sysfs_delbr(struct net_de
 #else
 
 #define br_sysfs_addif(p)	(0)
+#define br_sysfs_renameif(p)	(0)
 #define br_sysfs_addbr(dev)	(0)
 #define br_sysfs_delbr(dev)	do { } while(0)
 #endif /* CONFIG_SYSFS */
--- a/net/bridge/br_sysfs_if.c	2010-05-06 12:24:30.000000000 -0700
+++ b/net/bridge/br_sysfs_if.c	2010-05-06 12:48:07.747112472 -0700
@@ -246,7 +246,7 @@ const struct sysfs_ops brport_sysfs_ops 
 /*
  * Add sysfs entries to ethernet device added to a bridge.
  * Creates a brport subdirectory with bridge attributes.
- * Puts symlink in bridge's brport subdirectory
+ * Puts symlink in bridge's brif subdirectory
  */
 int br_sysfs_addif(struct net_bridge_port *p)
 {
@@ -257,15 +257,37 @@ int br_sysfs_addif(struct net_bridge_por
 	err = sysfs_create_link(&p->kobj, &br->dev->dev.kobj,
 				SYSFS_BRIDGE_PORT_LINK);
 	if (err)
-		goto out2;
+		return err;
 
 	for (a = brport_attrs; *a; ++a) {
 		err = sysfs_create_file(&p->kobj, &((*a)->attr));
 		if (err)
-			goto out2;
+			return err;
 	}
 
-	err = sysfs_create_link(br->ifobj, &p->kobj, p->dev->name);
-out2:
+	strlcpy(p->sysfs_name, p->dev->name, IFNAMSIZ);
+	return sysfs_create_link(br->ifobj, &p->kobj, p->sysfs_name);
+}
+
+/* Rename bridge's brif symlink */
+int br_sysfs_renameif(struct net_bridge_port *p)
+{
+	struct net_bridge *br = p->br;
+	int err;
+
+	/* If a rename fails, the rollback will cause another
+	 * rename call with the existing name.
+	 */
+	if (!strncmp(p->sysfs_name, p->dev->name, IFNAMSIZ))
+		return 0;
+
+	err = sysfs_rename_link(br->ifobj, &p->kobj,
+				p->sysfs_name, p->dev->name);
+	if (err)
+		netdev_notice(br->dev, "unable to rename link %s to %s",
+			      p->sysfs_name, p->dev->name);
+	else
+		strlcpy(p->sysfs_name, p->dev->name, IFNAMSIZ);
+
 	return err;
 }

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

end of thread, other threads:[~2010-05-06 19:51 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-06 18:32 [PATCH] bridge: update sysfs link names if port device names have changed Simon Arlott
2010-05-06 18:53 ` Stephen Hemminger
2010-05-06 19:01 ` [PATCH (v2)] " Simon Arlott
2010-05-06 19:11   ` Stephen Hemminger
2010-05-06 19:37     ` [PATCH (v4)] " Simon Arlott
2010-05-06 19:40       ` Stephen Hemminger
2010-05-06 19:50       ` [PATCH (v4+)] " Stephen Hemminger

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).