diff mbox

[net-next,v9,3/9] net: nl802154 - make add_iface take name assign type

Message ID 1405584370-30054-4-git-send-email-teg@jklm.no
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Tom Gundersen July 17, 2014, 8:06 a.m. UTC 
Signed-off-by: Tom Gundersen <teg@jklm.no>
Acked-by: Alexander Aring <alex.aring@gmail.com>
Cc: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Cc: linux-zigbee-devel@lists.sourceforge.net
---
 include/net/wpan-phy.h         | 4 +++-
 net/ieee802154/nl-phy.c        | 5 ++++-
 net/mac802154/ieee802154_dev.c | 7 ++++---
 3 files changed, 11 insertions(+), 5 deletions(-)

Comments

David Miller July 17, 2014, 11:19 p.m. UTC | #1 
From: Tom Gundersen <teg@jklm.no>
Date: Thu, 17 Jul 2014 10:06:04 +0200

> @@ -192,8 +193,10 @@ int ieee802154_add_iface(struct sk_buff *skb, struct genl_info *info)
>  		if (devname[nla_len(info->attrs[IEEE802154_ATTR_DEV_NAME]) - 1]
>  				!= '\0')
>  			return -EINVAL; /* phy name should be null-terminated */
> +		name_assign_type = NET_NAME_USER;
>  	} else  {
>  		devname = "wpan%d";
> +		name_assign_type = NET_NAME_ENUM;
>  	}
>  
>  	if (strlen(devname) >= IFNAMSIZ)

Just wondering what should happen if "%d" appears in a user provided name.

That would seem to be both USER and ENUM.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Aring July 18, 2014, 8:18 a.m. UTC | #2 
On Thu, Jul 17, 2014 at 04:19:31PM -0700, David Miller wrote:
> From: Tom Gundersen <teg@jklm.no>
> Date: Thu, 17 Jul 2014 10:06:04 +0200
> 
> > @@ -192,8 +193,10 @@ int ieee802154_add_iface(struct sk_buff *skb, struct genl_info *info)
> >  		if (devname[nla_len(info->attrs[IEEE802154_ATTR_DEV_NAME]) - 1]
> >  				!= '\0')
> >  			return -EINVAL; /* phy name should be null-terminated */
> > +		name_assign_type = NET_NAME_USER;
> >  	} else  {
> >  		devname = "wpan%d";
> > +		name_assign_type = NET_NAME_ENUM;
> >  	}
> >  
> >  	if (strlen(devname) >= IFNAMSIZ)
> 
> Just wondering what should happen if "%d" appears in a user provided name.
> 

I tested it. If I use a name like "foobar%d" then I get a foobar0,
foobar1 ... foobarX.

> That would seem to be both USER and ENUM.
> 

yes.


- Alex
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Laight July 18, 2014, 8:29 a.m. UTC | #3 
From: David Miller
> From: Tom Gundersen <teg@jklm.no>
> Date: Thu, 17 Jul 2014 10:06:04 +0200
> 
> > @@ -192,8 +193,10 @@ int ieee802154_add_iface(struct sk_buff *skb, struct genl_info *info)
> >  		if (devname[nla_len(info->attrs[IEEE802154_ATTR_DEV_NAME]) - 1]
> >  				!= '\0')
> >  			return -EINVAL; /* phy name should be null-terminated */
> > +		name_assign_type = NET_NAME_USER;
> >  	} else  {
> >  		devname = "wpan%d";
> > +		name_assign_type = NET_NAME_ENUM;
> >  	}
> >
> >  	if (strlen(devname) >= IFNAMSIZ)
> 
> Just wondering what should happen if "%d" appears in a user provided name.
> 
> That would seem to be both USER and ENUM.

Is a training %d detected by special code?
Or is the string used as a printf format?
If the latter is true what happens if the user provides foo%s

	David



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tom Gundersen July 18, 2014, 8:41 a.m. UTC | #4 
On Fri, Jul 18, 2014 at 1:19 AM, David Miller <davem@davemloft.net> wrote:
> From: Tom Gundersen <teg@jklm.no>
> Date: Thu, 17 Jul 2014 10:06:04 +0200
>
>> @@ -192,8 +193,10 @@ int ieee802154_add_iface(struct sk_buff *skb, struct genl_info *info)
>>               if (devname[nla_len(info->attrs[IEEE802154_ATTR_DEV_NAME]) - 1]
>>                               != '\0')
>>                       return -EINVAL; /* phy name should be null-terminated */
>> +             name_assign_type = NET_NAME_USER;
>>       } else  {
>>               devname = "wpan%d";
>> +             name_assign_type = NET_NAME_ENUM;
>>       }
>>
>>       if (strlen(devname) >= IFNAMSIZ)
>
> Just wondering what should happen if "%d" appears in a user provided name.
>
> That would seem to be both USER and ENUM.

Yes, this is a bit of a grey area.

I discussed this a bit with David Herrmann, and we landed on that
these names should be USER. As the user has explicitly asked for the
enumerated name, nobody should rename them to anything else (so ENUM
would have the wrong effect as it indicates that userspace should
rename the device), moreover, we should assume that the user knows
what they are doing, and that the enumerated names are fine in this
context.

Cheers,

Tom
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/net/wpan-phy.h b/include/net/wpan-phy.h
index 10ab0fc..bb07a1b 100644
--- a/include/net/wpan-phy.h
+++ b/include/net/wpan-phy.h
@@ -58,7 +58,9 @@  struct wpan_phy {
 	int idx;
 
 	struct net_device *(*add_iface)(struct wpan_phy *phy,
-					const char *name, int type);
+					const char *name,
+					unsigned char name_assign_type,
+					int type);
 	void (*del_iface)(struct wpan_phy *phy, struct net_device *dev);
 
 	int (*set_txpower)(struct wpan_phy *phy, int db);
diff --git a/net/ieee802154/nl-phy.c b/net/ieee802154/nl-phy.c
index 972baf8..5e1a28e 100644
--- a/net/ieee802154/nl-phy.c
+++ b/net/ieee802154/nl-phy.c
@@ -174,6 +174,7 @@  int ieee802154_add_iface(struct sk_buff *skb, struct genl_info *info)
 	struct wpan_phy *phy;
 	const char *name;
 	const char *devname;
+	unsigned char name_assign_type;
 	int rc = -ENOBUFS;
 	struct net_device *dev;
 	int type = __IEEE802154_DEV_INVALID;
@@ -192,8 +193,10 @@  int ieee802154_add_iface(struct sk_buff *skb, struct genl_info *info)
 		if (devname[nla_len(info->attrs[IEEE802154_ATTR_DEV_NAME]) - 1]
 				!= '\0')
 			return -EINVAL; /* phy name should be null-terminated */
+		name_assign_type = NET_NAME_USER;
 	} else  {
 		devname = "wpan%d";
+		name_assign_type = NET_NAME_ENUM;
 	}
 
 	if (strlen(devname) >= IFNAMSIZ)
@@ -227,7 +230,7 @@  int ieee802154_add_iface(struct sk_buff *skb, struct genl_info *info)
 		}
 	}
 
-	dev = phy->add_iface(phy, devname, type);
+	dev = phy->add_iface(phy, devname, name_assign_type, type);
 	if (IS_ERR(dev)) {
 		rc = PTR_ERR(dev);
 		goto nla_put_failure;
diff --git a/net/mac802154/ieee802154_dev.c b/net/mac802154/ieee802154_dev.c
index b36b2b9..cee12b2 100644
--- a/net/mac802154/ieee802154_dev.c
+++ b/net/mac802154/ieee802154_dev.c
@@ -159,7 +159,8 @@  mac802154_del_iface(struct wpan_phy *phy, struct net_device *dev)
 }
 
 static struct net_device *
-mac802154_add_iface(struct wpan_phy *phy, const char *name, int type)
+mac802154_add_iface(struct wpan_phy *phy, const char *name,
+		    unsigned char name_assign_type, int type)
 {
 	struct net_device *dev;
 	int err = -ENOMEM;
@@ -167,12 +168,12 @@  mac802154_add_iface(struct wpan_phy *phy, const char *name, int type)
 	switch (type) {
 	case IEEE802154_DEV_MONITOR:
 		dev = alloc_netdev(sizeof(struct mac802154_sub_if_data),
-				   name, NET_NAME_UNKNOWN,
+				   name, name_assign_type,
 				   mac802154_monitor_setup);
 		break;
 	case IEEE802154_DEV_WPAN:
 		dev = alloc_netdev(sizeof(struct mac802154_sub_if_data),
-				   name, NET_NAME_UNKNOWN,
+				   name, name_assign_type,
 				   mac802154_wpan_setup);
 		break;
 	default: