[PATCH v5 0/4] Provide netdev naming-policy via sysfs

[PATCH v5 0/4] Provide netdev naming-policy via sysfs

[David Herrmann]

Hi

This is v5 of the netdev naming-policy series. You can find v4 here:
  http://article.gmane.org/gmane.linux.kernel/1668161
Changes since v4:
  - none


This series implements a new sysfs attribute for netdevs called
"name_assign_type". It provides an integer that describes where an interface
name comes from. See Patch #1 for a description of this attribute. It is
modelled after the existing "addr_assign_type" attribute.

The main use-case is to allow udev to skip applying reliable ifnames to virtual
devices. For instance, if wifi-P2P devices are created, wpas already provides a
suitable naming-policy and udev shouldn't touch these devices. Same is true for
other virtual devices.
The idea is that if a device-name was provided by user-space, we should always
prefer fixing this naming-policy instead of making udev rename the device. For
kernel provided names that's hardly possible, though. Providing the
naming-policy source via sysfs is thus a simple way to see whether renames are
needed.

Additionally, this field allows to detect whether a netdev has been manually
renamed, which is quite useful for debugging and during crash-recovery.
Furthermore, it fixes real udev bugs if a netdev is already renamed in the
initrd and udev only runs in the real root. Detecting renames avoids overwriting
custom user provided names.

Thanks
David

David Herrmann (4):
  net: add name_assign_type netdev attribute
  mac80211: set NET_NAME_USER for user-space created ifs
  ath6kl: set NET_NAME_USER for P2P ifs
  brcmfmac: set NET_NAME_USER for P2P ifs

 drivers/net/wireless/ath/ath6kl/cfg80211.c    | 5 ++++-
 drivers/net/wireless/ath/ath6kl/cfg80211.h    | 1 +
 drivers/net/wireless/ath/ath6kl/core.c        | 4 ++--
 drivers/net/wireless/brcm80211/brcmfmac/p2p.c | 1 +
 include/linux/netdevice.h                     | 2 ++
 include/uapi/linux/netdevice.h                | 4 ++++
 net/core/dev.c                                | 7 +++++++
 net/core/net-sysfs.c                          | 2 ++
 net/core/rtnetlink.c                          | 2 ++
 net/mac80211/cfg.c                            | 2 +-
 net/mac80211/ieee80211_i.h                    | 1 +
 net/mac80211/iface.c                          | 2 ++
 net/mac80211/main.c                           | 2 +-
 13 files changed, 30 insertions(+), 5 deletions(-)

-- 
1.9.1

[PATCH v5 1/4] net: add name_assign_type netdev attribute

[David Herrmann]

The name_assign_type attribute gives hints where the interface name of a
given net-device comes from. Three different values are currently defined:
  NET_NAME_ENUM:
    This is the default. The ifname is provided by the kernel with an
    enumerated suffix. Names may be reused and unstable.
  NET_NAME_USER:
    The ifname was provided by user-space during net-device setup.
  NET_NAME_RENAMED:
    The net-device has been renamed via RTNL. Once this type is set, it
    cannot change again.

This attribute comes in handy for reliable net-device names. If an ifname
is provided by user-space, we can safely assume that the naming-policy
avoids reuse and is stable. Only if it was set by the kernel, the
interfaces might need to be renamed.

The NET_NAME_RENAMED value allows us to detect whether some-one else
already renamed the device, in which case we shouldn't touch it again. The
NET_NAME_USER value allows us to detect whether some other naming policy
created the device, in which case there's no need to rename it.

The most significant use-case is to detect virtual wifi-P2P devices, which
are named by wpa_supplicant et al. We shouldn't rename them as wpas
already provides a proper naming-policy.

Note that this patch only provides the core infrastructure. The different
net-dev types need to be manually fixed to use NET_NAME_USER instead of
the default (NET_NAME_ENUM). NET_NAME_ENUM is the least restrictive,
though, so it seems safe to use it as fallback for non-converted net-dev
types.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Acked-by: Tom Gundersen <teg@jklm.no>
---
 include/linux/netdevice.h      | 2 ++
 include/uapi/linux/netdevice.h | 4 ++++
 net/core/dev.c                 | 7 +++++++
 net/core/net-sysfs.c           | 2 ++
 net/core/rtnetlink.c           | 2 ++
 5 files changed, 17 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 4b6d12c..48e1c15 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1376,6 +1376,8 @@ struct net_device {
 	struct kset		*queues_kset;
 #endif
 
+	unsigned char		name_assign_type; /* name assignment type */
+
 	bool			uc_promisc;
 	unsigned int		promiscuity;
 	unsigned int		allmulti;
diff --git a/include/uapi/linux/netdevice.h b/include/uapi/linux/netdevice.h
index 6b9500b..ea963e4 100644
--- a/include/uapi/linux/netdevice.h
+++ b/include/uapi/linux/netdevice.h
@@ -36,6 +36,10 @@
 /* Initial net device group. All devices belong to group 0 by default. */
 #define INIT_NETDEV_GROUP	0
 
+/* interface name assignment types (sysfs name_assign_type attribute) */
+#define NET_NAME_ENUM		0	/* enumerated by kernel (default) */
+#define NET_NAME_USER		1	/* provided by user-space */
+#define NET_NAME_RENAMED	2	/* renamed by user-space */
 
 
 /* Media selection options. */
diff --git a/net/core/dev.c b/net/core/dev.c
index 55f8e64..ee0231e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1083,6 +1083,7 @@ static int dev_get_valid_name(struct net *net,
 int dev_change_name(struct net_device *dev, const char *newname)
 {
 	char oldname[IFNAMSIZ];
+	unsigned char old_assign_type;
 	int err = 0;
 	int ret;
 	struct net *net;
@@ -1109,10 +1110,14 @@ int dev_change_name(struct net_device *dev, const char *newname)
 		return err;
 	}
 
+	old_assign_type = dev->name_assign_type;
+	dev->name_assign_type = NET_NAME_RENAMED;
+
 rollback:
 	ret = device_rename(&dev->dev, dev->name);
 	if (ret) {
 		memcpy(dev->name, oldname, IFNAMSIZ);
+		dev->name_assign_type = old_assign_type;
 		write_seqcount_end(&devnet_rename_seq);
 		return ret;
 	}
@@ -1141,6 +1146,8 @@ rollback:
 			write_seqcount_begin(&devnet_rename_seq);
 			memcpy(dev->name, oldname, IFNAMSIZ);
 			memcpy(oldname, newname, IFNAMSIZ);
+			dev->name_assign_type = old_assign_type;
+			old_assign_type = NET_NAME_RENAMED;
 			goto rollback;
 		} else {
 			pr_err("%s: name change rollback failed: %d\n",
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index daed9a6..886403e 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -105,6 +105,7 @@ static ssize_t netdev_store(struct device *dev, struct device_attribute *attr,
 
 NETDEVICE_SHOW_RO(dev_id, fmt_hex);
 NETDEVICE_SHOW_RO(dev_port, fmt_dec);
+NETDEVICE_SHOW_RO(name_assign_type, fmt_dec);
 NETDEVICE_SHOW_RO(addr_assign_type, fmt_dec);
 NETDEVICE_SHOW_RO(addr_len, fmt_dec);
 NETDEVICE_SHOW_RO(iflink, fmt_dec);
@@ -377,6 +378,7 @@ static struct attribute *net_class_attrs[] = {
 	&dev_attr_dev_port.attr,
 	&dev_attr_iflink.attr,
 	&dev_attr_ifindex.attr,
+	&dev_attr_name_assign_type.attr,
 	&dev_attr_addr_assign_type.attr,
 	&dev_attr_addr_len.attr,
 	&dev_attr_link_mode.attr,
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index e7c6006..4dd2a64 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1967,6 +1967,8 @@ replay:
 		}
 
 		dev->ifindex = ifm->ifi_index;
+		if (tb[IFLA_IFNAME])
+			dev->name_assign_type = NET_NAME_USER;
 
 		if (ops->newlink) {
 			err = ops->newlink(net, dev, tb, data);
-- 
1.9.1

[PATCH v5 2/4] mac80211: set NET_NAME_USER for user-space created ifs

[David Herrmann]

The nl80211 interface allows creating new netdevs from user-space. The
name is *always* provided by user-space, so we should set NET_NAME_USER to
provide that information via sysfs. But we must not set it for the default
wlan%d names as these are kernel-provided names.

This allows udev to not rename dynamically created wifi devices (like wifi
P2P devices).

Cc: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Acked-by: Tom Gundersen <teg@jklm.no>
---
 net/mac80211/cfg.c         | 2 +-
 net/mac80211/ieee80211_i.h | 1 +
 net/mac80211/iface.c       | 2 ++
 net/mac80211/main.c        | 2 +-
 4 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index aaa59d7..804cf0a 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -31,7 +31,7 @@ static struct wireless_dev *ieee80211_add_iface(struct wiphy *wiphy,
 	struct ieee80211_sub_if_data *sdata;
 	int err;
 
-	err = ieee80211_if_add(local, name, &wdev, type, params);
+	err = ieee80211_if_add(local, name, NET_NAME_USER, &wdev, type, params);
 	if (err)
 		return ERR_PTR(err);
 
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 222c28b..1a938c3 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1470,6 +1470,7 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
 int ieee80211_iface_init(void);
 void ieee80211_iface_exit(void);
 int ieee80211_if_add(struct ieee80211_local *local, const char *name,
+		     unsigned char name_assign_type,
 		     struct wireless_dev **new_wdev, enum nl80211_iftype type,
 		     struct vif_params *params);
 int ieee80211_if_change_type(struct ieee80211_sub_if_data *sdata,
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index b8d331e..fe84853 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1574,6 +1574,7 @@ static void ieee80211_assign_perm_addr(struct ieee80211_local *local,
 }
 
 int ieee80211_if_add(struct ieee80211_local *local, const char *name,
+		     unsigned char name_assign_type,
 		     struct wireless_dev **new_wdev, enum nl80211_iftype type,
 		     struct vif_params *params)
 {
@@ -1617,6 +1618,7 @@ int ieee80211_if_add(struct ieee80211_local *local, const char *name,
 					+ IEEE80211_ENCRYPT_HEADROOM;
 		ndev->needed_tailroom = IEEE80211_ENCRYPT_TAILROOM;
 
+		ndev->name_assign_type = name_assign_type;
 		ret = dev_alloc_name(ndev, ndev->name);
 		if (ret < 0) {
 			free_netdev(ndev);
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index b055f6a5..ee32a83 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -1012,7 +1012,7 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
 
 	/* add one default STA interface if supported */
 	if (local->hw.wiphy->interface_modes & BIT(NL80211_IFTYPE_STATION)) {
-		result = ieee80211_if_add(local, "wlan%d", NULL,
+		result = ieee80211_if_add(local, "wlan%d", NET_NAME_ENUM, NULL,
 					  NL80211_IFTYPE_STATION, NULL);
 		if (result)
 			wiphy_warn(local->hw.wiphy,
-- 
1.9.1

[PATCH v5 3/4] ath6kl: set NET_NAME_USER for P2P ifs

[David Herrmann]

P2P netdevs and other devices that are created via nl80211 from user-space
have a name provided by user-space. Therefore, set NET_NAME_USER so this
is correctly shown in sysfs.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Acked-by: Kalle Valo <kvalo@qca.qualcomm.com>
Acked-by: Tom Gundersen <teg@jklm.no>
---
 drivers/net/wireless/ath/ath6kl/cfg80211.c | 5 ++++-
 drivers/net/wireless/ath/ath6kl/cfg80211.h | 1 +
 drivers/net/wireless/ath/ath6kl/core.c     | 4 ++--
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c
index c2c6f46..131d8ab 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -1512,7 +1512,8 @@ static struct wireless_dev *ath6kl_cfg80211_add_iface(struct wiphy *wiphy,
 		return ERR_PTR(-EINVAL);
 	}
 
-	wdev = ath6kl_interface_add(ar, name, type, if_idx, nw_type);
+	wdev = ath6kl_interface_add(ar, name, NET_NAME_USER, type,
+				    if_idx, nw_type);
 	if (!wdev)
 		return ERR_PTR(-ENOMEM);
 
@@ -3630,6 +3631,7 @@ void ath6kl_cfg80211_vif_cleanup(struct ath6kl_vif *vif)
 }
 
 struct wireless_dev *ath6kl_interface_add(struct ath6kl *ar, const char *name,
+					  unsigned char name_assign_type,
 					  enum nl80211_iftype type,
 					  u8 fw_vif_idx, u8 nw_type)
 {
@@ -3666,6 +3668,7 @@ struct wireless_dev *ath6kl_interface_add(struct ath6kl *ar, const char *name,
 			ndev->dev_addr[4] ^= 0x80;
 	}
 
+	ndev->name_assign_type = name_assign_type;
 	init_netdev(ndev);
 
 	ath6kl_init_control_info(vif);
diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.h b/drivers/net/wireless/ath/ath6kl/cfg80211.h
index b59becd..5aa57a7 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.h
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.h
@@ -25,6 +25,7 @@ enum ath6kl_cfg_suspend_mode {
 };
 
 struct wireless_dev *ath6kl_interface_add(struct ath6kl *ar, const char *name,
+					  unsigned char name_assign_type,
 					  enum nl80211_iftype type,
 					  u8 fw_vif_idx, u8 nw_type);
 void ath6kl_cfg80211_ch_switch_notify(struct ath6kl_vif *vif, int freq,
diff --git a/drivers/net/wireless/ath/ath6kl/core.c b/drivers/net/wireless/ath/ath6kl/core.c
index 4b46adb..3cc8145 100644
--- a/drivers/net/wireless/ath/ath6kl/core.c
+++ b/drivers/net/wireless/ath/ath6kl/core.c
@@ -195,8 +195,8 @@ int ath6kl_core_init(struct ath6kl *ar, enum ath6kl_htc_type htc_type)
 	rtnl_lock();
 
 	/* Add an initial station interface */
-	wdev = ath6kl_interface_add(ar, "wlan%d", NL80211_IFTYPE_STATION, 0,
-				    INFRA_NETWORK);
+	wdev = ath6kl_interface_add(ar, "wlan%d", NET_NAME_ENUM,
+				    NL80211_IFTYPE_STATION, 0, INFRA_NETWORK);
 
 	rtnl_unlock();
 
-- 
1.9.1

[PATCH v5 4/4] brcmfmac: set NET_NAME_USER for P2P ifs

[David Herrmann]

Netdevs created via nl80211 (currently only P2P ifs) have names provided
by user-space. Therefore, set the naming-policy to NET_NAME_USER so it is
correctly shown in sysfs.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Acked-by: Arend van Spriel <arend@broadcom.com>
Acked-by: Tom Gundersen <teg@jklm.no>
---
 drivers/net/wireless/brcm80211/brcmfmac/p2p.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
index f3445ac..f721ed6 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
@@ -2308,6 +2308,7 @@ struct wireless_dev *brcmf_p2p_add_vif(struct wiphy *wiphy, const char *name,
 		goto fail;
 	}
 
+	ifp->ndev->name_assign_type = NET_NAME_USER;
 	strncpy(ifp->ndev->name, name, sizeof(ifp->ndev->name) - 1);
 	err = brcmf_net_attach(ifp, true);
 	if (err) {
-- 
1.9.1

Re: [PATCH v5 0/4] Provide netdev naming-policy via sysfs

[David Miller]

From: David Herrmann <dh.herrmann@gmail.com>
Date: Wed, 26 Mar 2014 14:05:13 +0100

> The main use-case is to allow udev to skip applying reliable ifnames to virtual
> devices. For instance, if wifi-P2P devices are created, wpas already provides a
> suitable naming-policy and udev shouldn't touch these devices. Same is true for
> other virtual devices.

This makes no sense at all.

If udev should avoid applying names to wifi-P2P devices, that policy can
be instituted completely inside of udev.  There is no need whatsoever
for kernel support.

udev can look at the device type, and policies can be defined that key
off of that device type, entirely in userspace.

I'm not applying this patch series, sorry.

Re: [PATCH v5 0/4] Provide netdev naming-policy via sysfs

[David Herrmann]

Hi

On Fri, Mar 28, 2014 at 7:54 PM, David Miller <davem@davemloft.net> wrote:
> udev can look at the device type, and policies can be defined that key
> off of that device type, entirely in userspace.

Regarding wifi-P2P devices, udev can indeed use nl80211 to get the
exact IF-type. An attribute would be much easier to read out, but I'm
ok if we don't want to replicate that information.

However, as mentioned in the discussions on v1, there're more
use-cases than that. Imagine a 3rd party initrd renames a
network-device early, if udev runs in the main system, we would rename
the device again. If we could detect the rename via NET_NAME_RENAMED,
we wouldn't break such setups. The only reason to rename devices in
udev is to get reliable names. If someone else already renamed a
device, we always expect them to provide better names than we do, so
udev should only touch devices that have kernel-enumerated names.
Of course, this can be fixed by requiring all the initrd developers to
somehow tag devices as already renamed. But this would replicate
information that the kernel already has.

Another example are dynamically created devices via RTM_NEWLINK. The
if-names are provided by user-space and udev should never try to do
any magic renaming on them (why would we? you can just easily fix the
code that created the device).

So it turns out, all we want to know is whether an interface-name was
chosen by the kernel via plain, simple enumeration, or whether it has
a user-space origin. Only in the former case we want to rename the
device. The name_assign_type attribute tells us exactly where a name
comes from and thus avoids maintaining blacklists in user-space that
skip renaming on such devices. I'm sorry, but I cannot see why such
blacklists (or whitelists) are preferred over a sysfs attribute.

Thanks
David

Re: [PATCH v5 0/4] Provide netdev naming-policy via sysfs

[David Miller]

From: David Herrmann <dh.herrmann@gmail.com>
Date: Fri, 28 Mar 2014 20:34:47 +0100

> However, as mentioned in the discussions on v1, there're more
> use-cases than that. Imagine a 3rd party initrd renames a
> network-device early, if udev runs in the main system, we would rename
> the device again. If we could detect the rename via NET_NAME_RENAMED,
> we wouldn't break such setups. The only reason to rename devices in
> udev is to get reliable names. If someone else already renamed a
> device, we always expect them to provide better names than we do, so
> udev should only touch devices that have kernel-enumerated names.
> Of course, this can be fixed by requiring all the initrd developers to
> somehow tag devices as already renamed. But this would replicate
> information that the kernel already has.

You want to use the kernel to facilitate communication between two
userland components.

Imagine if we had to do that for everything.

Really, it's not a good reason, sorry.

Re: [PATCH v5 0/4] Provide netdev naming-policy via sysfs

[Tom Gundersen]

On Fri, Mar 28, 2014 at 7:54 PM, David Miller <davem@davemloft.net> wrote:
> From: David Herrmann <dh.herrmann@gmail.com>
> Date: Wed, 26 Mar 2014 14:05:13 +0100
>
>> The main use-case is to allow udev to skip applying reliable ifnames to virtual
>> devices. For instance, if wifi-P2P devices are created, wpas already provides a
>> suitable naming-policy and udev shouldn't touch these devices. Same is true for
>> other virtual devices.
>
> This makes no sense at all.
>
> If udev should avoid applying names to wifi-P2P devices, that policy can
> be instituted completely inside of udev.  There is no need whatsoever
> for kernel support.
>
> udev can look at the device type, and policies can be defined that key
> off of that device type, entirely in userspace.

Doing this in userspace sounds really wrong and fragile.

In the case of wifi-P2P we could make it work, but for every type of
device that is added to the kernel which is named from userspace, we
would have to play catch-up in udev (and even after we do, new kernels
on old userspace will never work in the expected way).

Moreover, there are of course cases where knowing the type of device
is not sufficient. Take 'ip link add type bridge' vs. 'ip link add dev
bride5 type bridge'. They may both create a new bridge called
'bride5'. In the first case, we may reasonably want a policy in udev
to rename the device to something better, but in the second case we
probably want to leave it alone.

>From userspace there is no way (that I know of) to determine if the
name of a given device was picked by the kernel (i.e., more or less
randomly) or by the userspace process that created it (i.e., probably
for a good reason, so we should not touch it).

Also the problems with third-party processes renaming devices which
David outlined is a real problem that we run into in practice. We
could of course dream up mechanisms for these processes to communicate
to udev that they already renamed the device (and we have), but people
will keep getting this wrong, so asking the kernel for this
information just makes so much more sense.

These patches seem to solve all the problems with network interface
renaming really nicely once and for all, so would be a real shame if
they don't go in.

Cheers,

Tom

Re: [PATCH v5 0/4] Provide netdev naming-policy via sysfs

[David Miller]

From: Tom Gundersen <teg@jklm.no>
Date: Fri, 28 Mar 2014 21:51:53 +0100

> On Fri, Mar 28, 2014 at 7:54 PM, David Miller <davem@davemloft.net> wrote:
>> From: David Herrmann <dh.herrmann@gmail.com>
>> Date: Wed, 26 Mar 2014 14:05:13 +0100
>>
>>> The main use-case is to allow udev to skip applying reliable ifnames to virtual
>>> devices. For instance, if wifi-P2P devices are created, wpas already provides a
>>> suitable naming-policy and udev shouldn't touch these devices. Same is true for
>>> other virtual devices.
>>
>> This makes no sense at all.
>>
>> If udev should avoid applying names to wifi-P2P devices, that policy can
>> be instituted completely inside of udev.  There is no need whatsoever
>> for kernel support.
>>
>> udev can look at the device type, and policies can be defined that key
>> off of that device type, entirely in userspace.
> 
> Doing this in userspace sounds really wrong and fragile.
> 
> In the case of wifi-P2P we could make it work, but for every type of
> device that is added to the kernel which is named from userspace, we
> would have to play catch-up in udev (and even after we do, new kernels
> on old userspace will never work in the expected way).

Chronically we are finding hackish ways for seperate components in
userspace to coordinate their actions.

You're right, doing this for every device type for every single attribute
that might better.... sucks.

What's really needed is generic ways for seperate userspace components
to coordinate with eachother on issues like this.

Re: [PATCH v5 0/4] Provide netdev naming-policy via sysfs

[Tom Gundersen]

On Fri, Mar 28, 2014 at 10:21 PM, David Miller <davem@davemloft.net> wrote:
> From: Tom Gundersen <teg@jklm.no>
> Date: Fri, 28 Mar 2014 21:51:53 +0100
>
>> On Fri, Mar 28, 2014 at 7:54 PM, David Miller <davem@davemloft.net> wrote:
>>> From: David Herrmann <dh.herrmann@gmail.com>
>>> Date: Wed, 26 Mar 2014 14:05:13 +0100
>>>
>>>> The main use-case is to allow udev to skip applying reliable ifnames to virtual
>>>> devices. For instance, if wifi-P2P devices are created, wpas already provides a
>>>> suitable naming-policy and udev shouldn't touch these devices. Same is true for
>>>> other virtual devices.
>>>
>>> This makes no sense at all.
>>>
>>> If udev should avoid applying names to wifi-P2P devices, that policy can
>>> be instituted completely inside of udev.  There is no need whatsoever
>>> for kernel support.
>>>
>>> udev can look at the device type, and policies can be defined that key
>>> off of that device type, entirely in userspace.
>>
>> Doing this in userspace sounds really wrong and fragile.
>>
>> In the case of wifi-P2P we could make it work, but for every type of
>> device that is added to the kernel which is named from userspace, we
>> would have to play catch-up in udev (and even after we do, new kernels
>> on old userspace will never work in the expected way).
>
> Chronically we are finding hackish ways for seperate components in
> userspace to coordinate their actions.
>
> You're right, doing this for every device type for every single attribute
> that might better.... sucks.
>
> What's really needed is generic ways for seperate userspace components
> to coordinate with eachother on issues like this.

You mean coordinate with each other in userspace? If so, I still don't
see how this can ever be anything else than fragile. It will depend on
each userspace component actually opting in to whatever scheme we
devise, and does so correctly.

The kernel is the only one who can know where the names came from in a
reliable way (no matter how crappy the userspace component who
originally created or renamed the devices is).

In udev we can ensure that what we do ourselves is sane, and we are
happy to trust that what the kernel does is sane (and that the info it
exposes to us is correct). However, we cannot really rely on each of
the myriad of different components who may in various ways
create/rename netdevs will manage to correctly hook into some
synchronisation mechanism.

Given that the kernel already has all the necessary info, and that the
patch to expose it is so trivial, and that it is absolutely not clear
at all that this can even be done in userspace (let alone being done
in a sane way), I still struggle to see the reason for rejecting the
patch...

Or am I missing some obvious way we can solve this in userspace?

Cheers,

Tom

Re: [PATCH v5 0/4] Provide netdev naming-policy via sysfs

[David Miller]

From: Tom Gundersen <teg@jklm.no>
Date: Fri, 28 Mar 2014 23:39:57 +0100

> You mean coordinate with each other in userspace? If so, I still don't
> see how this can ever be anything else than fragile. It will depend on
> each userspace component actually opting in to whatever scheme we
> devise, and does so correctly.

Isn't that essentially what dbus is?

A way for seperate userland components to coordinate their
activities?

I just simply don't like all of these ramdom keys getting
added all over the place to guide udev behavior wrt. some
other entity.

Re: [PATCH v5 0/4] Provide netdev naming-policy via sysfs

[Tom Gundersen]

On Sat, Mar 29, 2014 at 2:42 AM, David Miller <davem@davemloft.net> wrote:
> From: Tom Gundersen <teg@jklm.no>
> Date: Fri, 28 Mar 2014 23:39:57 +0100
>
>> You mean coordinate with each other in userspace? If so, I still don't
>> see how this can ever be anything else than fragile. It will depend on
>> each userspace component actually opting in to whatever scheme we
>> devise, and does so correctly.
>
> Isn't that essentially what dbus is?
>
> A way for seperate userland components to coordinate their
> activities?

The protocol is not the problem. The issue is that this information
only exists reliably in the kernel, so we need to get it from there
somehow.

> I just simply don't like all of these ramdom keys getting
> added all over the place to guide udev behavior wrt. some
> other entity.

Well, in this case udev is sort of doing a bit of the kernel's work,
and it needs some information from the kernel to do it reliably.
Another approach would be for the kernel to just assign predictable
interface names to devices to begin with and we would never have to
touch them. You have all the information so that would be relatively
easy.

The issue I see with that is that there are several ways to generate
predictable names, and the user may want to chose between them, so
this is arguably policy that should not be in the kernel. If you don't
think that's a problem, I'd be happy to submit a patch to move all
this logic from udev to the kernel, just let me know how you see it.

Cheers,

Tom

Re: [PATCH v5 0/4] Provide netdev naming-policy via sysfs

[David Miller]

From: Tom Gundersen <teg@jklm.no>
Date: Sat, 29 Mar 2014 10:46:02 +0100

> The issue I see with that is that there are several ways to generate
> predictable names, and the user may want to chose between them, so
> this is arguably policy that should not be in the kernel. If you don't
> think that's a problem, I'd be happy to submit a patch to move all
> this logic from udev to the kernel, just let me know how you see it.

Unfortunately, we have already allowed this kind of thing for setting
MAC addresses so I guess I'll have to allow your changes to be applied.

But before I ask you to resubmit, do you have full buy-in from the
udev folks for this facility?

Thanks.

Re: [PATCH v5 0/4] Provide netdev naming-policy via sysfs

[Kay Sievers]

On Sat, Mar 29, 2014 at 8:37 PM, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
> From: Tom Gundersen <teg-B22kvLQNl6c@public.gmane.org>
> Date: Sat, 29 Mar 2014 10:46:02 +0100
>
>> The issue I see with that is that there are several ways to generate
>> predictable names, and the user may want to chose between them, so
>> this is arguably policy that should not be in the kernel. If you don't
>> think that's a problem, I'd be happy to submit a patch to move all
>> this logic from udev to the kernel, just let me know how you see it.
>
> Unfortunately, we have already allowed this kind of thing for setting
> MAC addresses so I guess I'll have to allow your changes to be applied.
>
> But before I ask you to resubmit, do you have full buy-in from the
> udev folks for this facility?

Absolutely, it is very useful to have.

Today, there is no way for userland to distinguish explicitly
requested and predictable names provided by tools, from unpredictable
names composed by the kernel itself by adding an unpredictable
enumeration number.

It allows us the work around the common problems in general purpose
Linux, where we have no strong dependencies, not necessarily central
management, and loose coupling between components.

This information will allow us to apply best-effort or common sense to
the default udev device name policies, like:
  - leave the name alone if someone has asked explicitly for that name
  - leave the name alone if someone else has already renamed it after
its first creation

So, having that information in /sys would be very welcome from udev's
point of view.

Thanks,
Kay
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html