All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Provide netdev naming-policy via sysfs
@ 2014-02-27 14:47 David Herrmann
  2014-02-27 14:47 ` [PATCH 1/4] net: add name_assign_type netdev attribute David Herrmann
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: David Herrmann @ 2014-02-27 14:47 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Kay Sievers, Tom Gundersen, Johannes berg,
	linux-kernel, linux-wireless, David Herrmann

Hi

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.

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                     | 6 ++++++
 net/core/dev.c                                | 7 +++++++
 net/core/net-sysfs.c                          | 2 ++
 net/core/rtnetlink.c                          | 2 ++
 net/mac80211/iface.c                          | 1 +
 9 files changed, 26 insertions(+), 3 deletions(-)

-- 
1.9.0


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

* [PATCH 1/4] net: add name_assign_type netdev attribute
  2014-02-27 14:47 [PATCH 0/4] Provide netdev naming-policy via sysfs David Herrmann
@ 2014-02-27 14:47 ` David Herrmann
  2014-02-27 14:47 ` [PATCH 2/4] mac80211: set NET_NAME_USER for user-space created ifs David Herrmann
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: David Herrmann @ 2014-02-27 14:47 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Kay Sievers, Tom Gundersen, Johannes berg,
	linux-kernel, linux-wireless, 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>
---
 include/linux/netdevice.h | 6 ++++++
 net/core/dev.c            | 7 +++++++
 net/core/net-sysfs.c      | 2 ++
 net/core/rtnetlink.c      | 2 ++
 4 files changed, 17 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index e8eeebd..d3a11c8 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -74,6 +74,11 @@ void netdev_set_default_ethtool_ops(struct net_device *dev,
 #define NET_RX_SUCCESS		0	/* keep 'em coming, baby */
 #define NET_RX_DROP		1	/* packet dropped */
 
+/* interface name assignment types */
+#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 */
+
 /*
  * Transmit return codes: transmit return codes originate from three different
  * namespaces:
@@ -1165,6 +1170,7 @@ struct net_device {
 	 * of the interface.
 	 */
 	char			name[IFNAMSIZ];
+	unsigned char		name_assign_type; /* name assignment type */
 
 	/* device name hash chain, please keep it close to name[] */
 	struct hlist_node	name_hlist;
diff --git a/net/core/dev.c b/net/core/dev.c
index b1b0c8d..3608fb2 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 9388624..72a8ce5 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -104,6 +104,7 @@ static ssize_t netdev_store(struct device *dev, struct device_attribute *attr,
 }
 
 NETDEVICE_SHOW_RO(dev_id, fmt_hex);
+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);
@@ -375,6 +376,7 @@ static struct attribute *net_class_attrs[] = {
 	&dev_attr_dev_id.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 1a0dac2..e10baa0 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1962,6 +1962,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.0


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

* [PATCH 2/4] mac80211: set NET_NAME_USER for user-space created ifs
  2014-02-27 14:47 [PATCH 0/4] Provide netdev naming-policy via sysfs David Herrmann
  2014-02-27 14:47 ` [PATCH 1/4] net: add name_assign_type netdev attribute David Herrmann
@ 2014-02-27 14:47 ` David Herrmann
  2014-02-27 14:53   ` Johannes Berg
  2014-02-27 14:47 ` [PATCH 3/4] ath6kl: set NET_NAME_USER for P2P ifs David Herrmann
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: David Herrmann @ 2014-02-27 14:47 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Kay Sievers, Tom Gundersen, Johannes berg,
	linux-kernel, linux-wireless, David Herrmann, Johannes Berg

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.

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>
---
 net/mac80211/iface.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index ce1c443..35561bd 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1620,6 +1620,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 = NET_NAME_USER;
 		ret = dev_alloc_name(ndev, ndev->name);
 		if (ret < 0) {
 			free_netdev(ndev);
-- 
1.9.0


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

* [PATCH 3/4] ath6kl: set NET_NAME_USER for P2P ifs
  2014-02-27 14:47 [PATCH 0/4] Provide netdev naming-policy via sysfs David Herrmann
  2014-02-27 14:47 ` [PATCH 1/4] net: add name_assign_type netdev attribute David Herrmann
  2014-02-27 14:47 ` [PATCH 2/4] mac80211: set NET_NAME_USER for user-space created ifs David Herrmann
@ 2014-02-27 14:47 ` David Herrmann
  2014-02-27 15:27   ` Kalle Valo
  2014-02-27 14:47 ` [PATCH 4/4] brcmfmac: " David Herrmann
  2014-02-27 15:47 ` [PATCH 0/4] Provide netdev naming-policy via sysfs Tom Gundersen
  4 siblings, 1 reply; 14+ messages in thread
From: David Herrmann @ 2014-02-27 14:47 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Kay Sievers, Tom Gundersen, Johannes berg,
	linux-kernel, linux-wireless, David Herrmann, Kalle Valo

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.

Cc: Kalle Valo <kvalo@qca.qualcomm.com>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 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 fd4c89d..74598ed 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -1516,7 +1516,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);
 
@@ -3625,6 +3626,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)
 {
@@ -3661,6 +3663,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.0


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

* [PATCH 4/4] brcmfmac: set NET_NAME_USER for P2P ifs
  2014-02-27 14:47 [PATCH 0/4] Provide netdev naming-policy via sysfs David Herrmann
                   ` (2 preceding siblings ...)
  2014-02-27 14:47 ` [PATCH 3/4] ath6kl: set NET_NAME_USER for P2P ifs David Herrmann
@ 2014-02-27 14:47 ` David Herrmann
  2014-02-27 17:30   ` Arend van Spriel
  2014-02-27 15:47 ` [PATCH 0/4] Provide netdev naming-policy via sysfs Tom Gundersen
  4 siblings, 1 reply; 14+ messages in thread
From: David Herrmann @ 2014-02-27 14:47 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Kay Sievers, Tom Gundersen, Johannes berg,
	linux-kernel, linux-wireless, David Herrmann, Arend van Spriel

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.

Cc: Arend van Spriel <arend@broadcom.com>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 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 fc4f98b..6af4d26 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
@@ -2310,6 +2310,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.0


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

* Re: [PATCH 2/4] mac80211: set NET_NAME_USER for user-space created ifs
  2014-02-27 14:47 ` [PATCH 2/4] mac80211: set NET_NAME_USER for user-space created ifs David Herrmann
@ 2014-02-27 14:53   ` Johannes Berg
  2014-02-27 14:56     ` David Herrmann
  0 siblings, 1 reply; 14+ messages in thread
From: Johannes Berg @ 2014-02-27 14:53 UTC (permalink / raw)
  To: David Herrmann
  Cc: netdev, David S. Miller, Kay Sievers, Tom Gundersen,
	linux-kernel, linux-wireless

Given the premise of your patches,

> +++ b/net/mac80211/iface.c
> @@ -1620,6 +1620,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 = NET_NAME_USER;

this is wrong because we call this in main.c with a "wlan%d" argument
for the name, which gets expanded.

johannes


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

* Re: [PATCH 2/4] mac80211: set NET_NAME_USER for user-space created ifs
  2014-02-27 14:53   ` Johannes Berg
@ 2014-02-27 14:56     ` David Herrmann
  2014-02-27 16:18       ` Johannes Berg
  0 siblings, 1 reply; 14+ messages in thread
From: David Herrmann @ 2014-02-27 14:56 UTC (permalink / raw)
  To: Johannes Berg
  Cc: netdev, David S. Miller, Kay Sievers, Tom Gundersen,
	linux-kernel, linux-wireless

Hi

On Thu, Feb 27, 2014 at 3:53 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> Given the premise of your patches,
>
>> +++ b/net/mac80211/iface.c
>> @@ -1620,6 +1620,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 = NET_NAME_USER;
>
> this is wrong because we call this in main.c with a "wlan%d" argument
> for the name, which gets expanded.

Whoops, yeah, same issue as with ath6kl where I fixed it with a
separate "name_assign_type" argument. I will do the same here for v2.

Thanks
David

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

* Re: [PATCH 3/4] ath6kl: set NET_NAME_USER for P2P ifs
  2014-02-27 14:47 ` [PATCH 3/4] ath6kl: set NET_NAME_USER for P2P ifs David Herrmann
@ 2014-02-27 15:27   ` Kalle Valo
  2014-02-27 16:13     ` Kalle Valo
  0 siblings, 1 reply; 14+ messages in thread
From: Kalle Valo @ 2014-02-27 15:27 UTC (permalink / raw)
  To: David Herrmann
  Cc: netdev, David S. Miller, Kay Sievers, Tom Gundersen,
	Johannes berg, linux-kernel, linux-wireless

David Herrmann <dh.herrmann@gmail.com> writes:

> 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.
>
> Cc: Kalle Valo <kvalo@qca.qualcomm.com>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>

Looks good to me.

Acked-by: Kalle Valo <kvalo@qca.qualcomm.com>

Can I take this to my ath.git tree?

-- 
Kalle Valo

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

* Re: [PATCH 0/4] Provide netdev naming-policy via sysfs
  2014-02-27 14:47 [PATCH 0/4] Provide netdev naming-policy via sysfs David Herrmann
                   ` (3 preceding siblings ...)
  2014-02-27 14:47 ` [PATCH 4/4] brcmfmac: " David Herrmann
@ 2014-02-27 15:47 ` Tom Gundersen
  2014-03-01 14:19   ` Tom Gundersen
  4 siblings, 1 reply; 14+ messages in thread
From: Tom Gundersen @ 2014-02-27 15:47 UTC (permalink / raw)
  To: David Herrmann
  Cc: netdev, David S. Miller, Kay Sievers, Johannes berg, LKML,
	Linux Wireless List

On Thu, Feb 27, 2014 at 3:47 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> 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.

Moreover, it would be useful for udev to reliably know if some other
userspace process already renamed a device, so we know not to touch
it. This can easily happen for instance if some renaming happens in
the initrd or from script called from udev rules.

Acked-by: Tom Gundersen <teg@jklm.no>

> 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                     | 6 ++++++
>  net/core/dev.c                                | 7 +++++++
>  net/core/net-sysfs.c                          | 2 ++
>  net/core/rtnetlink.c                          | 2 ++
>  net/mac80211/iface.c                          | 1 +
>  9 files changed, 26 insertions(+), 3 deletions(-)
>
> --
> 1.9.0
>

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

* Re: [PATCH 3/4] ath6kl: set NET_NAME_USER for P2P ifs
  2014-02-27 15:27   ` Kalle Valo
@ 2014-02-27 16:13     ` Kalle Valo
  0 siblings, 0 replies; 14+ messages in thread
From: Kalle Valo @ 2014-02-27 16:13 UTC (permalink / raw)
  To: David Herrmann
  Cc: netdev, David S. Miller, Kay Sievers, Tom Gundersen,
	Johannes berg, linux-kernel, linux-wireless

Kalle Valo <kvalo@qca.qualcomm.com> writes:

> David Herrmann <dh.herrmann@gmail.com> writes:
>
>> 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.
>>
>> Cc: Kalle Valo <kvalo@qca.qualcomm.com>
>> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
>
> Looks good to me.
>
> Acked-by: Kalle Valo <kvalo@qca.qualcomm.com>
>
> Can I take this to my ath.git tree?

Nevermind, I saw the first patch now. Just CCing random patches like
this is confusing for the recipients, better to CC the full patchset.

-- 
Kalle Valo

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

* Re: [PATCH 2/4] mac80211: set NET_NAME_USER for user-space created ifs
  2014-02-27 14:56     ` David Herrmann
@ 2014-02-27 16:18       ` Johannes Berg
  2014-02-27 17:14         ` David Herrmann
  0 siblings, 1 reply; 14+ messages in thread
From: Johannes Berg @ 2014-02-27 16:18 UTC (permalink / raw)
  To: David Herrmann
  Cc: netdev, David S. Miller, Kay Sievers, Tom Gundersen,
	linux-kernel, linux-wireless

On Thu, 2014-02-27 at 15:56 +0100, David Herrmann wrote:

> >> +++ b/net/mac80211/iface.c
> >> @@ -1620,6 +1620,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 = NET_NAME_USER;
> >
> > this is wrong because we call this in main.c with a "wlan%d" argument
> > for the name, which gets expanded.
> 
> Whoops, yeah, same issue as with ath6kl where I fixed it with a
> separate "name_assign_type" argument. I will do the same here for v2.

I'm not sure that helps - the user could potentially pass "foobar%d" as
the interface name and get a number assigned by the kernel. In that case
you probably *don't* want to set it to "name assigned by user"?

Maybe this whole thing can just be handled in __dev_alloc_name() since
no kernel user should pass just a string without "%d" in it.

johannes


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

* Re: [PATCH 2/4] mac80211: set NET_NAME_USER for user-space created ifs
  2014-02-27 16:18       ` Johannes Berg
@ 2014-02-27 17:14         ` David Herrmann
  0 siblings, 0 replies; 14+ messages in thread
From: David Herrmann @ 2014-02-27 17:14 UTC (permalink / raw)
  To: Johannes Berg
  Cc: netdev, David S. Miller, Kay Sievers, Tom Gundersen,
	linux-kernel, linux-wireless

Hi

On Thu, Feb 27, 2014 at 5:18 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Thu, 2014-02-27 at 15:56 +0100, David Herrmann wrote:
>
>> >> +++ b/net/mac80211/iface.c
>> >> @@ -1620,6 +1620,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 = NET_NAME_USER;
>> >
>> > this is wrong because we call this in main.c with a "wlan%d" argument
>> > for the name, which gets expanded.
>>
>> Whoops, yeah, same issue as with ath6kl where I fixed it with a
>> separate "name_assign_type" argument. I will do the same here for v2.
>
> I'm not sure that helps - the user could potentially pass "foobar%d" as
> the interface name and get a number assigned by the kernel. In that case
> you probably *don't* want to set it to "name assigned by user"?
>
> Maybe this whole thing can just be handled in __dev_alloc_name() since
> no kernel user should pass just a string without "%d" in it.

Nope, I still want to set NET_NAME_USER in that case. It's not about
the %d suffix, it's about who chooses the suffix. If wpas decides to
use p2p-wlanX-%d, this is fine, as it's a virtual device where stable
IDs are not required at all.

Thanks
David

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

* Re: [PATCH 4/4] brcmfmac: set NET_NAME_USER for P2P ifs
  2014-02-27 14:47 ` [PATCH 4/4] brcmfmac: " David Herrmann
@ 2014-02-27 17:30   ` Arend van Spriel
  0 siblings, 0 replies; 14+ messages in thread
From: Arend van Spriel @ 2014-02-27 17:30 UTC (permalink / raw)
  To: David Herrmann, netdev
  Cc: David S. Miller, Kay Sievers, Tom Gundersen, Johannes berg,
	linux-kernel, linux-wireless

On 02/27/2014 03:47 PM, David Herrmann wrote:
> 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.
> 
- Cc: Arend van Spriel <arend@broadcom.com>
+ Acked-by: Arend van Spriel <arend@broadcom.com>

> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---
>  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 fc4f98b..6af4d26 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/p2p.c
> @@ -2310,6 +2310,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) {
> 


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

* Re: [PATCH 0/4] Provide netdev naming-policy via sysfs
  2014-02-27 15:47 ` [PATCH 0/4] Provide netdev naming-policy via sysfs Tom Gundersen
@ 2014-03-01 14:19   ` Tom Gundersen
  0 siblings, 0 replies; 14+ messages in thread
From: Tom Gundersen @ 2014-03-01 14:19 UTC (permalink / raw)
  To: David Herrmann
  Cc: netdev, David S. Miller, Kay Sievers, Johannes berg, LKML,
	Linux Wireless List

On Thu, Feb 27, 2014 at 4:47 PM, Tom Gundersen <teg@jklm.no> wrote:
> On Thu, Feb 27, 2014 at 3:47 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
>> 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.
>
> Moreover, it would be useful for udev to reliably know if some other
> userspace process already renamed a device, so we know not to touch
> it. This can easily happen for instance if some renaming happens in
> the initrd or from script called from udev rules.

Incidentally, just after writing this email I ran into precisely this
problem due to buggy udev rules in a third-party package. The result
was that the upstream NIC naming rules were unable to detect that a
given NIC had already been renamed and ended up renaming them a second
time (hence wrecking havoc).

The problem was easy enough to fix, but with these patches we would be
able to avoid the issue altogether, so I'm looking forward to fixing
up udev to using this interface.

> Acked-by: Tom Gundersen <teg@jklm.no>

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

end of thread, other threads:[~2014-03-01 14:19 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-27 14:47 [PATCH 0/4] Provide netdev naming-policy via sysfs David Herrmann
2014-02-27 14:47 ` [PATCH 1/4] net: add name_assign_type netdev attribute David Herrmann
2014-02-27 14:47 ` [PATCH 2/4] mac80211: set NET_NAME_USER for user-space created ifs David Herrmann
2014-02-27 14:53   ` Johannes Berg
2014-02-27 14:56     ` David Herrmann
2014-02-27 16:18       ` Johannes Berg
2014-02-27 17:14         ` David Herrmann
2014-02-27 14:47 ` [PATCH 3/4] ath6kl: set NET_NAME_USER for P2P ifs David Herrmann
2014-02-27 15:27   ` Kalle Valo
2014-02-27 16:13     ` Kalle Valo
2014-02-27 14:47 ` [PATCH 4/4] brcmfmac: " David Herrmann
2014-02-27 17:30   ` Arend van Spriel
2014-02-27 15:47 ` [PATCH 0/4] Provide netdev naming-policy via sysfs Tom Gundersen
2014-03-01 14:19   ` Tom Gundersen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.