[PATCH] Add symlink in /sys/class/net for interface altnames

[PATCH] Add symlink in /sys/class/net for interface altnames

[Jorge Merlino]

Currently interface altnames behave almost the same as the interface
principal name. One difference is that the not have a symlink in
/sys/class/net as the principal has.
This was mentioned as a TODO item in the original commit:
https://lore.kernel.org/netdev/20190719110029.29466-1-jiri@resnulli.us
This patch adds that symlink when an altname is created and removes it
when the altname is deleted.

Signed-off-by: Jorge Merlino <jorge.merlino@canonical.com>
---
 drivers/base/core.c    | 22 ++++++++++++++++++++++
 include/linux/device.h |  3 +++
 net/core/dev.c         | 11 +++++++++++
 3 files changed, 36 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index e54a10b5d..165f51438 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -4223,6 +4223,28 @@ void root_device_unregister(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(root_device_unregister);
 
+/**
+ * device_add_altname_symlink - add a symlink in /sys/class/net for a device
+ * altname
+ * @dev: device getting a new altname
+ * @altname: new altname
+ */
+int device_add_altname_symlink(struct device *dev, const char *altname)
+{
+	return sysfs_create_link(&dev->class->p->subsys.kobj, &dev->kobj,
+			altname);
+}
+
+/**
+ * device_remove_altname_symlink - remove device altname symlink from
+ * /sys/class/net
+ * @dev: device losing an altname
+ * @altname: removed altname
+ */
+void device_remove_altname_symlink(struct device *dev, const char *altname)
+{
+	sysfs_delete_link(&dev->class->p->subsys.kobj, &dev->kobj, altname);
+}
 
 static void device_create_release(struct device *dev)
 {
diff --git a/include/linux/device.h b/include/linux/device.h
index 1508e637b..658d4d743 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -986,6 +986,9 @@ struct device *__root_device_register(const char *name, struct module *owner);
 
 void root_device_unregister(struct device *root);
 
+int device_add_altname_symlink(struct device *dev, const char *altname);
+void device_remove_altname_symlink(struct device *dev, const char *altname);
+
 static inline void *dev_get_platdata(const struct device *dev)
 {
 	return dev->platform_data;
diff --git a/net/core/dev.c b/net/core/dev.c
index 253584777..b40ed0b21 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -150,6 +150,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/prandom.h>
 #include <linux/once_lite.h>
+#include <linux/device.h>
 
 #include "dev.h"
 #include "net-sysfs.h"
@@ -328,6 +329,7 @@ int netdev_name_node_alt_create(struct net_device *dev, const char *name)
 {
 	struct netdev_name_node *name_node;
 	struct net *net = dev_net(dev);
+	int ret;
 
 	name_node = netdev_name_node_lookup(net, name);
 	if (name_node)
@@ -339,6 +341,11 @@ int netdev_name_node_alt_create(struct net_device *dev, const char *name)
 	/* The node that holds dev->name acts as a head of per-device list. */
 	list_add_tail(&name_node->list, &dev->name_node->list);
 
+#ifdef CONFIG_SYSFS
+	ret = device_add_altname_symlink(&dev->dev, name);
+	if (ret)
+		netdev_info(dev, "Unable to create symlink for altname: %d\n", ret);
+#endif
 	return 0;
 }
 
@@ -366,6 +373,10 @@ int netdev_name_node_alt_destroy(struct net_device *dev, const char *name)
 
 	__netdev_name_node_alt_destroy(name_node);
 
+#ifdef CONFIG_SYSFS
+	device_remove_altname_symlink(&dev->dev, name);
+#endif
+
 	return 0;
 }
 
-- 
2.37.2

Re: [PATCH] Add symlink in /sys/class/net for interface altnames

[Greg Kroah-Hartman]

On Mon, Mar 13, 2023 at 01:49:03PM -0300, Jorge Merlino wrote:
> Currently interface altnames behave almost the same as the interface
> principal name. One difference is that the not have a symlink in
> /sys/class/net as the principal has.
> This was mentioned as a TODO item in the original commit:
> https://lore.kernel.org/netdev/20190719110029.29466-1-jiri@resnulli.us
> This patch adds that symlink when an altname is created and removes it
> when the altname is deleted.
> 
> Signed-off-by: Jorge Merlino <jorge.merlino@canonical.com>
> ---
>  drivers/base/core.c    | 22 ++++++++++++++++++++++
>  include/linux/device.h |  3 +++
>  net/core/dev.c         | 11 +++++++++++
>  3 files changed, 36 insertions(+)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index e54a10b5d..165f51438 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -4223,6 +4223,28 @@ void root_device_unregister(struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(root_device_unregister);
>  
> +/**
> + * device_add_altname_symlink - add a symlink in /sys/class/net for a device

If this is only for networking devices, why are you accepting any device
pointer?

> + * altname
> + * @dev: device getting a new altname
> + * @altname: new altname
> + */
> +int device_add_altname_symlink(struct device *dev, const char *altname)
> +{
> +	return sysfs_create_link(&dev->class->p->subsys.kobj, &dev->kobj,

That's a deep -> chain, are you _SURE_ that is going to work properly?
You just want a link in the subsystem directory, so why not pass in the
class/subsystem instead?


> +			altname);
> +}
> +
> +/**
> + * device_remove_altname_symlink - remove device altname symlink from
> + * /sys/class/net
> + * @dev: device losing an altname
> + * @altname: removed altname
> + */
> +void device_remove_altname_symlink(struct device *dev, const char *altname)
> +{
> +	sysfs_delete_link(&dev->class->p->subsys.kobj, &dev->kobj, altname);

Same here, why not pass in the class?

> +}
>  
>  static void device_create_release(struct device *dev)
>  {
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 1508e637b..658d4d743 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -986,6 +986,9 @@ struct device *__root_device_register(const char *name, struct module *owner);
>  
>  void root_device_unregister(struct device *root);
>  
> +int device_add_altname_symlink(struct device *dev, const char *altname);
> +void device_remove_altname_symlink(struct device *dev, const char *altname);
> +
>  static inline void *dev_get_platdata(const struct device *dev)
>  {
>  	return dev->platform_data;
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 253584777..b40ed0b21 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -150,6 +150,7 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/prandom.h>
>  #include <linux/once_lite.h>
> +#include <linux/device.h>
>  
>  #include "dev.h"
>  #include "net-sysfs.h"
> @@ -328,6 +329,7 @@ int netdev_name_node_alt_create(struct net_device *dev, const char *name)
>  {
>  	struct netdev_name_node *name_node;
>  	struct net *net = dev_net(dev);
> +	int ret;
>  
>  	name_node = netdev_name_node_lookup(net, name);
>  	if (name_node)
> @@ -339,6 +341,11 @@ int netdev_name_node_alt_create(struct net_device *dev, const char *name)
>  	/* The node that holds dev->name acts as a head of per-device list. */
>  	list_add_tail(&name_node->list, &dev->name_node->list);
>  
> +#ifdef CONFIG_SYSFS
> +	ret = device_add_altname_symlink(&dev->dev, name);

Why do you need a #ifdef?  Put the proper #ifdef in the .h file please.

> +	if (ret)
> +		netdev_info(dev, "Unable to create symlink for altname: %d\n", ret);

info level for an error?

> +#endif
>  	return 0;
>  }
>  
> @@ -366,6 +373,10 @@ int netdev_name_node_alt_destroy(struct net_device *dev, const char *name)
>  
>  	__netdev_name_node_alt_destroy(name_node);
>  
> +#ifdef CONFIG_SYSFS
> +	device_remove_altname_symlink(&dev->dev, name);
> +#endif

Again, no #ifdef should be needed.

thanks,

greg k-h

Re: [PATCH] Add symlink in /sys/class/net for interface altnames

[Greg Kroah-Hartman]

On Mon, Mar 13, 2023 at 01:49:03PM -0300, Jorge Merlino wrote:
> Currently interface altnames behave almost the same as the interface
> principal name. One difference is that the not have a symlink in
> /sys/class/net as the principal has.
> This was mentioned as a TODO item in the original commit:
> https://lore.kernel.org/netdev/20190719110029.29466-1-jiri@resnulli.us
> This patch adds that symlink when an altname is created and removes it
> when the altname is deleted.
> 
> Signed-off-by: Jorge Merlino <jorge.merlino@canonical.com>
> ---
>  drivers/base/core.c    | 22 ++++++++++++++++++++++
>  include/linux/device.h |  3 +++
>  net/core/dev.c         | 11 +++++++++++
>  3 files changed, 36 insertions(+)
> 

You also forgot the Documentation/ABI/ update for the new symlink :(

Re: [PATCH] Add symlink in /sys/class/net for interface altnames

[Stephen Hemminger]

On Mon, 13 Mar 2023 13:49:03 -0300
Jorge Merlino <jorge.merlino@canonical.com> wrote:

> Currently interface altnames behave almost the same as the interface
> principal name. One difference is that the not have a symlink in
> /sys/class/net as the principal has.
> This was mentioned as a TODO item in the original commit:
> https://lore.kernel.org/netdev/20190719110029.29466-1-jiri@resnulli.us
> This patch adds that symlink when an altname is created and removes it
> when the altname is deleted.
> 
> Signed-off-by: Jorge Merlino <jorge.merlino@canonical.com>

What about if altname is changed?

Re: [PATCH] Add symlink in /sys/class/net for interface altnames

[Jakub Kicinski]

On Mon, 13 Mar 2023 13:49:03 -0300 Jorge Merlino wrote:
> Currently interface altnames behave almost the same as the interface
> principal name. One difference is that the not have a symlink in
> /sys/class/net as the principal has.
> This was mentioned as a TODO item in the original commit:
> https://lore.kernel.org/netdev/20190719110029.29466-1-jiri@resnulli.us
> This patch adds that symlink when an altname is created and removes it
> when the altname is deleted.

I think this is risky. Altnames are added by systemd on unsuspecting
systems, and I've seen configurations systems which assume that all
/sys/class/net entries are separate devices (basically counting how 
many NICs machine has).

I'm afraid it's too late.