[PATCH 1/2] Revert "wireless: Keep phy name consistent across module reloads."

[PATCH 1/2] Revert "wireless: Keep phy name consistent across module reloads."

[greearb]

From: Ben Greear <greearb@candelatech.com>

This reverts commit a6ab8e2903d4416a53e3bcc97ae2d3148a36c5df.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 8226ba7... 9c21ebf... M	net/wireless/core.c
 net/wireless/core.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 8226ba7..9c21ebf 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -319,7 +319,8 @@ static void cfg80211_event_work(struct work_struct *work)
 
 struct wiphy *wiphy_new(const struct cfg80211_ops *ops, int sizeof_priv)
 {
-	int i;
+	static int wiphy_counter;
+
 	struct cfg80211_registered_device *rdev;
 	int alloc_size;
 
@@ -341,18 +342,12 @@ struct wiphy *wiphy_new(const struct cfg80211_ops *ops, int sizeof_priv)
 
 	mutex_lock(&cfg80211_mutex);
 
-	/* 64k wiphy devices is enough for anyone! */
-	for (i = 0; i < 0xFFFF; i++) {
-		if (!cfg80211_rdev_by_wiphy_idx(i))
-			break;
-	}
-	if (i == 0xFFFF)
-		i = -1; /* invalid */
-	rdev->wiphy_idx = i;
+	rdev->wiphy_idx = wiphy_counter++;
 
 	if (unlikely(!wiphy_idx_valid(rdev->wiphy_idx))) {
+		wiphy_counter--;
 		mutex_unlock(&cfg80211_mutex);
-		/* ugh, too many devices already! */
+		/* ugh, wrapped! */
 		kfree(rdev);
 		return NULL;
 	}
-- 
1.7.2.3

[PATCH 2/2] wireless: Use first phyX name available when registering phy devices.

[greearb]

From: Ben Greear <greearb@candelatech.com>

Choose first available phyX name when creating phy devices.  This
means that reloading a wifi driver will not cause a change in the
name of it's phy device.

Also, allow users to rename a phy to any un-used name, including
phy%d.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 9c21ebf... 1684ad9... M	net/wireless/core.c
 net/wireless/core.c |   54 ++++++++++++++++++++++++++++----------------------
 1 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 9c21ebf..1684ad9 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -178,26 +178,10 @@ int cfg80211_dev_rename(struct cfg80211_registered_device *rdev,
 			char *newname)
 {
 	struct cfg80211_registered_device *rdev2;
-	int wiphy_idx, taken = -1, result, digits;
+	int result;
 
 	assert_cfg80211_lock();
 
-	/* prohibit calling the thing phy%d when %d is not its number */
-	sscanf(newname, PHY_NAME "%d%n", &wiphy_idx, &taken);
-	if (taken == strlen(newname) && wiphy_idx != rdev->wiphy_idx) {
-		/* count number of places needed to print wiphy_idx */
-		digits = 1;
-		while (wiphy_idx /= 10)
-			digits++;
-		/*
-		 * deny the name if it is phy<idx> where <idx> is printed
-		 * without leading zeroes. taken == strlen(newname) here
-		 */
-		if (taken == strlen(PHY_NAME) + digits)
-			return -EINVAL;
-	}
-
-
 	/* Ignore nop renames */
 	if (strcmp(newname, dev_name(&rdev->wiphy.dev)) == 0)
 		return 0;
@@ -205,7 +189,7 @@ int cfg80211_dev_rename(struct cfg80211_registered_device *rdev,
 	/* Ensure another device does not already have this name. */
 	list_for_each_entry(rdev2, &cfg80211_rdev_list, list)
 		if (strcmp(newname, dev_name(&rdev2->wiphy.dev)) == 0)
-			return -EINVAL;
+			return -EEXIST;
 
 	result = device_rename(&rdev->wiphy.dev, newname);
 	if (result)
@@ -320,9 +304,11 @@ static void cfg80211_event_work(struct work_struct *work)
 struct wiphy *wiphy_new(const struct cfg80211_ops *ops, int sizeof_priv)
 {
 	static int wiphy_counter;
-
-	struct cfg80211_registered_device *rdev;
+	int i;
+	struct cfg80211_registered_device *rdev, *rdev2;
 	int alloc_size;
+	char nname[IFNAMSIZ + 1];
+	bool found = false;
 
 	WARN_ON(ops->add_key && (!ops->del_key || !ops->set_default_key));
 	WARN_ON(ops->auth && (!ops->assoc || !ops->deauth || !ops->disassoc));
@@ -346,16 +332,36 @@ struct wiphy *wiphy_new(const struct cfg80211_ops *ops, int sizeof_priv)
 
 	if (unlikely(!wiphy_idx_valid(rdev->wiphy_idx))) {
 		wiphy_counter--;
+		goto too_many_devs;
+	}
+
+	/* 64k wiphy devices is enough for anyone! */
+	for (i = 0; i < 0xFFFF; i++) {
+		found = false;
+		snprintf(nname, sizeof(nname)-1, PHY_NAME "%d", i);
+		nname[sizeof(nname)-1] = 0;
+		list_for_each_entry(rdev2, &cfg80211_rdev_list, list)
+			if (strcmp(nname, dev_name(&rdev2->wiphy.dev)) == 0) {
+				found = true;
+				break;
+			}
+
+		if (!found)
+			break;
+	}
+
+	if (unlikely(found)) {
+too_many_devs:
 		mutex_unlock(&cfg80211_mutex);
-		/* ugh, wrapped! */
+		/* ugh, too many devices already! */
 		kfree(rdev);
 		return NULL;
 	}
 
-	mutex_unlock(&cfg80211_mutex);
-
 	/* give it a proper name */
-	dev_set_name(&rdev->wiphy.dev, PHY_NAME "%d", rdev->wiphy_idx);
+	dev_set_name(&rdev->wiphy.dev, "%s", nname);
+
+	mutex_unlock(&cfg80211_mutex);
 
 	mutex_init(&rdev->mtx);
 	mutex_init(&rdev->devlist_mtx);
-- 
1.7.2.3

Re: [PATCH 2/2] wireless: Use first phyX name available when registering phy devices.

[Johannes Berg]

On Mon, 2010-09-27 at 09:07 -0700, greearb@gmail.com wrote:

> @@ -346,16 +332,36 @@ struct wiphy *wiphy_new(const struct cfg80211_ops *ops, int sizeof_priv)
>  
>  	if (unlikely(!wiphy_idx_valid(rdev->wiphy_idx))) {
>  		wiphy_counter--;
> +		goto too_many_devs;
> +	}

I believe the other path can also reduce the counter again, since it's
another failure case? Not that I'm really worried about it though.

> +	/* 64k wiphy devices is enough for anyone! */
> +	for (i = 0; i < 0xFFFF; i++) {

Come to think of it, this could be "i <= wiphy_counter", since in N
devices, only N different names can be in use, so checking N+1 names
will be sufficient, right?

johannes

Re: [PATCH 2/2] wireless: Use first phyX name available when registering phy devices.

[Ben Greear]

On 09/27/2010 09:17 AM, Johannes Berg wrote:
> On Mon, 2010-09-27 at 09:07 -0700, greearb@gmail.com wrote:
>
>> @@ -346,16 +332,36 @@ struct wiphy *wiphy_new(const struct cfg80211_ops *ops, int sizeof_priv)
>>
>>   	if (unlikely(!wiphy_idx_valid(rdev->wiphy_idx))) {
>>   		wiphy_counter--;
>> +		goto too_many_devs;
>> +	}
>
> I believe the other path can also reduce the counter again, since it's
> another failure case? Not that I'm really worried about it though.
>
>> +	/* 64k wiphy devices is enough for anyone! */
>> +	for (i = 0; i<  0xFFFF; i++) {
>
> Come to think of it, this could be "i<= wiphy_counter", since in N
> devices, only N different names can be in use, so checking N+1 names
> will be sufficient, right?

I think it doesn't matter so much either way.  If we want to get really
paranoid about this, then we need to hash lookup of phy by index and name
and probably allow some reuse of phy indexes in case someone wants to reload
a module 4 billion times.

But, since current use is probably < 2 phys in most cases, and less than 500 in
all cases, I think we could just leave the code as is.  I'd guess the only reason
to change this is if we ever get usable virtual phys (netdev had the same migration
when VLANs started being useful and suddenly it was OK to have several thousand
interfaces.)

That said, I'll redo the patch if you want these changes.

Thanks,
Ben

>
> johannes
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com