2023-03-13 16:50:32

by Jorge Merlino

[permalink] [raw]
Subject: [PATCH] Add symlink in /sys/class/net for interface altnames

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/[email protected]
This patch adds that symlink when an altname is created and removes it
when the altname is deleted.

Signed-off-by: Jorge Merlino <[email protected]>
---
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



2023-03-13 17:04:41

by Greg Kroah-Hartman

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

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/[email protected]
> This patch adds that symlink when an altname is created and removes it
> when the altname is deleted.
>
> Signed-off-by: Jorge Merlino <[email protected]>
> ---
> 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

2023-03-13 17:05:22

by Greg Kroah-Hartman

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

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/[email protected]
> This patch adds that symlink when an altname is created and removes it
> when the altname is deleted.
>
> Signed-off-by: Jorge Merlino <[email protected]>
> ---
> 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 :(

2023-03-13 17:25:31

by Stephen Hemminger

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

On Mon, 13 Mar 2023 13:49:03 -0300
Jorge Merlino <[email protected]> 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/[email protected]
> This patch adds that symlink when an altname is created and removes it
> when the altname is deleted.
>
> Signed-off-by: Jorge Merlino <[email protected]>

What about if altname is changed?

2023-03-13 21:39:29

by Jakub Kicinski

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

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/[email protected]
> 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.