Re: [wireless] wireless: Keep phy name consistent across module reloads.

[Ben Greear <greearb@candelatech.com>]

[-- Attachment #1: Type: text/plain, Size: 3219 bytes --]

On 09/24/2010 02:31 PM, Johannes Berg wrote:
> On Fri, 2010-09-24 at 14:25 -0700, Ben Greear wrote:
>
>>> Actually, right now it doesn't even allow you to use a "phy%d" name,
>>> which is because I don't want (sysfs, debugfs) errors coming from doing
>>> "iw phy0 set name phy1" and then plugging in a new device ... In doing
>>> that, I manage to avoid having to allocate two different numbers...
>>
>> I tried rename few months ago, to rename back to phy0, and it failed
>> for no obvious reason..so I just assumed rename didn't work at all.
>> Would you like a patch to print something in kernel logs when someone
>> tries to rename to phy%d?
>
> Well, if you want to work on getting an unused phy%d name this would be
> kinda pointless, no?
>
>>> I suppose it'd be possible to treat this more like interfaces. Want to
>>> also make it per net namespace? ;-)
>>
>> Probably best for all involved if I don't mess with namespaces right now,
>> but I'll be happy to work on (or test) a patch to find the first un-used phyX name,
>> if that's what you mean.
>
> I think that'd be OK, even if maybe at this point a little unexpected?

Here's a patch for consideration.  It's on top of my other patch,
but I can re-do it against a tree with that reverted if you prefer.

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 8226ba7..312a44b 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -319,9 +319,12 @@ 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;
  	int i;
-	struct cfg80211_registered_device *rdev;
+	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));
@@ -341,26 +344,38 @@ struct wiphy *wiphy_new(const struct cfg80211_ops *ops, int sizeof_priv)

  	mutex_lock(&cfg80211_mutex);

+	rdev->wiphy_idx = wiphy_counter++;
+
+	if (unlikely(!wiphy_idx_valid(rdev->wiphy_idx)))
+		goto too_many_devs;
+
  	/* 64k wiphy devices is enough for anyone! */
  	for (i = 0; i < 0xFFFF; i++) {
-		if (!cfg80211_rdev_by_wiphy_idx(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 (i == 0xFFFF)
-		i = -1; /* invalid */
-	rdev->wiphy_idx = i;

-	if (unlikely(!wiphy_idx_valid(rdev->wiphy_idx))) {
+	if (unlikely(found)) {
+too_many_devs:
  		mutex_unlock(&cfg80211_mutex);
  		/* 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);


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


[-- Attachment #2: phy_name.patch --]
[-- Type: text/plain, Size: 1842 bytes --]

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 8226ba7..312a44b 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -319,9 +319,12 @@ 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;
 	int i;
-	struct cfg80211_registered_device *rdev;
+	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));
@@ -341,26 +344,38 @@ struct wiphy *wiphy_new(const struct cfg80211_ops *ops, int sizeof_priv)
 
 	mutex_lock(&cfg80211_mutex);
 
+	rdev->wiphy_idx = wiphy_counter++;
+
+	if (unlikely(!wiphy_idx_valid(rdev->wiphy_idx)))
+		goto too_many_devs;
+
 	/* 64k wiphy devices is enough for anyone! */
 	for (i = 0; i < 0xFFFF; i++) {
-		if (!cfg80211_rdev_by_wiphy_idx(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 (i == 0xFFFF)
-		i = -1; /* invalid */
-	rdev->wiphy_idx = i;
 
-	if (unlikely(!wiphy_idx_valid(rdev->wiphy_idx))) {
+	if (unlikely(found)) {
+too_many_devs:
 		mutex_unlock(&cfg80211_mutex);
 		/* 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);