diff mbox

[v2,1/6] cfg80211: support creating wiphy with suggested name.

Message ID 1413845378-27837-1-git-send-email-greearb@candelatech.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Ben Greear Oct. 20, 2014, 10:49 p.m. UTC 
From: Ben Greear <greearb@candelatech.com>

Kernel will attempt to use the name if it is supplied,
but if name cannot be used for some reason, the default
phyX name will be used instead.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 include/net/cfg80211.h | 22 +++++++++++++++++++++-
 net/wireless/core.c    | 45 ++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 63 insertions(+), 4 deletions(-)

Comments

Johannes Berg Oct. 21, 2014, 7:31 p.m. UTC | #1 
On Mon, 2014-10-20 at 15:49 -0700, greearb@candelatech.com wrote:

> + * @requested_name Request a particular name.

missing colon, missing documentation that it can be NULL (for no
request)

also missing in the documentation is the name change of the function


>  	if (unlikely(rdev->wiphy_idx < 0)) {
> +		/* TODO:  Fix this someday. */

That doesn't belong into this patch :)

>  	/* give it a proper name */
> -	dev_set_name(&rdev->wiphy.dev, PHY_NAME "%d", rdev->wiphy_idx);
> +	if (requested_name && requested_name[0]) {
> +		struct cfg80211_registered_device *rdev2;
> +		int wiphy_idx, taken = -1, result, digits;
> +
> +		/* Code below is from cfg80211_dev_rename */

can you refactor this then please?

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 Oct. 21, 2014, 8:10 p.m. UTC | #2 
On 10/21/2014 12:31 PM, Johannes Berg wrote:

>>  	/* give it a proper name */
>> -	dev_set_name(&rdev->wiphy.dev, PHY_NAME "%d", rdev->wiphy_idx);
>> +	if (requested_name && requested_name[0]) {
>> +		struct cfg80211_registered_device *rdev2;
>> +		int wiphy_idx, taken = -1, result, digits;
>> +
>> +		/* Code below is from cfg80211_dev_rename */
> 
> can you refactor this then please?

I'll see what I can do on this, as well as take care of
the rest of the comments.

Thanks,
Ben
Ben Greear Oct. 22, 2014, 4:07 p.m. UTC | #3 
On 10/21/2014 01:10 PM, Ben Greear wrote:
> On 10/21/2014 12:31 PM, Johannes Berg wrote:
> 
>>>  	/* give it a proper name */
>>> -	dev_set_name(&rdev->wiphy.dev, PHY_NAME "%d", rdev->wiphy_idx);
>>> +	if (requested_name && requested_name[0]) {
>>> +		struct cfg80211_registered_device *rdev2;
>>> +		int wiphy_idx, taken = -1, result, digits;
>>> +
>>> +		/* Code below is from cfg80211_dev_rename */
>>
>> can you refactor this then please?
> 
> I'll see what I can do on this, as well as take care of
> the rest of the comments.

I was thinking, this could be made simpler if we reduced restrictions on
the naming.  Basically, let user (re)name the wiphy however they want, as
long as it name is not currently in use.

Might require a bit of extra locking and checking instead of just assuming the
atomic-inc and phy%d is unique, but maybe that is worth the tradeoff?

I'll get started on trying to refactor with existing constraints, but
let me know how you feel about relaxing the restrictions...

Thanks,
Ben

> 
> Thanks,
> Ben
>
diff mbox

Patch

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index ed896c0..5252f80 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -3193,6 +3193,7 @@  static inline const char *wiphy_name(const struct wiphy *wiphy)
  *
  * @ops: The configuration operations for this device
  * @sizeof_priv: The size of the private area to allocate
+ * @requested_name Request a particular name.
  *
  * Create a new wiphy and associate the given operations with it.
  * @sizeof_priv bytes are allocated for private use.
@@ -3200,7 +3201,26 @@  static inline const char *wiphy_name(const struct wiphy *wiphy)
  * Return: A pointer to the new wiphy. This pointer must be
  * assigned to each netdev's ieee80211_ptr for proper operation.
  */
-struct wiphy *wiphy_new(const struct cfg80211_ops *ops, int sizeof_priv);
+struct wiphy *wiphy_new_nm(const struct cfg80211_ops *ops, int sizeof_priv,
+			   const char *requested_name);
+
+/**
+ * wiphy_new - create a new wiphy for use with cfg80211
+ *
+ * @ops: The configuration operations for this device
+ * @sizeof_priv: The size of the private area to allocate
+ *
+ * Create a new wiphy and associate the given operations with it.
+ * @sizeof_priv bytes are allocated for private use.
+ *
+ * Return: A pointer to the new wiphy. This pointer must be
+ * assigned to each netdev's ieee80211_ptr for proper operation.
+ */
+static inline struct wiphy *wiphy_new(const struct cfg80211_ops *ops,
+				      int sizeof_priv)
+{
+	return wiphy_new_nm(ops, sizeof_priv, NULL);
+}
 
 /**
  * wiphy_register - register a wiphy with cfg80211
diff --git a/net/wireless/core.c b/net/wireless/core.c
index f52a4cd..2013edb 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -309,7 +309,8 @@  static void cfg80211_destroy_iface_wk(struct work_struct *work)
 
 /* exported functions */
 
-struct wiphy *wiphy_new(const struct cfg80211_ops *ops, int sizeof_priv)
+struct wiphy *wiphy_new_nm(const struct cfg80211_ops *ops, int sizeof_priv,
+			   const char *requested_name)
 {
 	static atomic_t wiphy_counter = ATOMIC_INIT(0);
 
@@ -336,6 +337,7 @@  struct wiphy *wiphy_new(const struct cfg80211_ops *ops, int sizeof_priv)
 	rdev->wiphy_idx = atomic_inc_return(&wiphy_counter);
 
 	if (unlikely(rdev->wiphy_idx < 0)) {
+		/* TODO:  Fix this someday. */
 		/* ugh, wrapped! */
 		atomic_dec(&wiphy_counter);
 		kfree(rdev);
@@ -346,7 +348,44 @@  struct wiphy *wiphy_new(const struct cfg80211_ops *ops, int sizeof_priv)
 	rdev->wiphy_idx--;
 
 	/* give it a proper name */
-	dev_set_name(&rdev->wiphy.dev, PHY_NAME "%d", rdev->wiphy_idx);
+	if (requested_name && requested_name[0]) {
+		struct cfg80211_registered_device *rdev2;
+		int wiphy_idx, taken = -1, result, digits;
+
+		/* Code below is from cfg80211_dev_rename */
+		/* prohibit calling the thing phy%d when %d is not its number */
+		sscanf(requested_name, PHY_NAME "%d%n", &wiphy_idx, &taken);
+		if (taken == strlen(requested_name) &&
+		    wiphy_idx != rdev->wiphy_idx) {
+			/* count number of places needed to print wiphy_idx */
+			digits = 1;
+			while (wiphy_idx /= 10)
+				digits++;
+			/*
+			 * deny name if it is phy<idx> where <idx> is printed
+			 * without leading zeroes. taken == strlen(newname) here
+			 */
+			if (taken == strlen(PHY_NAME) + digits)
+				goto use_default_name;
+		}
+
+		rtnl_lock();
+		/* Ensure another device does not already have this name. */
+		list_for_each_entry(rdev2, &cfg80211_rdev_list, list)
+			if (strcmp(requested_name, dev_name(&rdev2->wiphy.dev))
+			    == 0) {
+				rtnl_unlock();
+				goto use_default_name;
+			}
+
+		result = dev_set_name(&rdev->wiphy.dev, requested_name);
+		rtnl_unlock();
+		if (result)
+			goto use_default_name;
+	} else {
+use_default_name:
+		dev_set_name(&rdev->wiphy.dev, PHY_NAME "%d", rdev->wiphy_idx);
+	}
 
 	INIT_LIST_HEAD(&rdev->wdev_list);
 	INIT_LIST_HEAD(&rdev->beacon_registrations);
@@ -406,7 +445,7 @@  struct wiphy *wiphy_new(const struct cfg80211_ops *ops, int sizeof_priv)
 
 	return &rdev->wiphy;
 }
-EXPORT_SYMBOL(wiphy_new);
+EXPORT_SYMBOL(wiphy_new_nm);
 
 static int wiphy_verify_combinations(struct wiphy *wiphy)
 {