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

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

[greearb]

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(-)

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)
 {
-- 
1.7.11.7

[PATCH v2 2/6] mac80211: allow creating wiphy devices with suggested name.

[greearb]

From: Ben Greear <greearb@candelatech.com>

Support creating wiphy devices with an optional name.
This will be used by hwsim to have better automated control
over virtual radio creation/deletion.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 include/net/mac80211.h | 25 ++++++++++++++++++++++++-
 net/mac80211/main.c    |  9 +++++----
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 9bb2fc7..a206a56 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -3069,11 +3069,34 @@ struct ieee80211_ops {
  *
  * @priv_data_len: length of private data
  * @ops: callbacks for this device
+ * @requested_name:  Requested name for this device.
  *
  * Return: A pointer to the new hardware device, or %NULL on error.
  */
+struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,
+					   const struct ieee80211_ops *ops,
+					   const char *requested_name);
+
+/**
+ * ieee80211_alloc_hw -  Allocate a new hardware device
+ *
+ * This must be called once for each hardware device. The returned pointer
+ * must be used to refer to this device when calling other functions.
+ * mac80211 allocates a private data area for the driver pointed to by
+ * @priv in &struct ieee80211_hw, the size of this area is given as
+ * @priv_data_len.
+ *
+ * @priv_data_len: length of private data
+ * @ops: callbacks for this device
+ *
+ * Return: A pointer to the new hardware device, or %NULL on error.
+ */
+static inline
 struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
-					const struct ieee80211_ops *ops);
+					const struct ieee80211_ops *ops)
+{
+	return ieee80211_alloc_hw_nm(priv_data_len, ops, NULL);
+}
 
 /**
  * ieee80211_register_hw - Register hardware device
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 9e322dc..b189122 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -478,8 +478,9 @@ static const struct ieee80211_vht_cap mac80211_vht_capa_mod_mask = {
 	},
 };
 
-struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
-					const struct ieee80211_ops *ops)
+struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,
+					   const struct ieee80211_ops *ops,
+					   const char *requested_name)
 {
 	struct ieee80211_local *local;
 	int priv_size, i;
@@ -519,7 +520,7 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
 	 */
 	priv_size = ALIGN(sizeof(*local), NETDEV_ALIGN) + priv_data_len;
 
-	wiphy = wiphy_new(&mac80211_config_ops, priv_size);
+	wiphy = wiphy_new_nm(&mac80211_config_ops, priv_size, requested_name);
 
 	if (!wiphy)
 		return NULL;
@@ -649,7 +650,7 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
 
 	return &local->hw;
 }
-EXPORT_SYMBOL(ieee80211_alloc_hw);
+EXPORT_SYMBOL(ieee80211_alloc_hw_nm);
 
 static int ieee80211_init_cipher_suites(struct ieee80211_local *local)
 {
-- 
1.7.11.7

[PATCH v2 3/6] mac80211-hwsim: support creating radios with specific name.

[greearb]

From: Ben Greear <greearb@candelatech.com>

Otherwise, it can be very difficult to know which is which
if you are trying to do detailed testing.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 drivers/net/wireless/mac80211_hwsim.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 00c7b3c..ef793d5 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -2023,7 +2023,7 @@ static int mac80211_hwsim_create_radio(int channels, const char *reg_alpha2,
 				       const struct ieee80211_regdomain *regd,
 				       bool reg_strict, bool p2p_device,
 				       bool use_chanctx, bool destroy_on_close,
-				       u32 portid)
+				       u32 portid, char *hwname)
 {
 	int err;
 	u8 addr[ETH_ALEN];
@@ -2042,7 +2042,7 @@ static int mac80211_hwsim_create_radio(int channels, const char *reg_alpha2,
 
 	if (use_chanctx)
 		ops = &mac80211_hwsim_mchan_ops;
-	hw = ieee80211_alloc_hw(sizeof(*data), ops);
+	hw = ieee80211_alloc_hw_nm(sizeof(*data), ops, hwname);
 	if (!hw) {
 		printk(KERN_DEBUG "mac80211_hwsim: ieee80211_alloc_hw failed\n");
 		err = -ENOMEM;
@@ -2516,10 +2516,14 @@ static int hwsim_create_radio_nl(struct sk_buff *msg, struct genl_info *info)
 	bool p2p_device = info->attrs[HWSIM_ATTR_SUPPORT_P2P_DEVICE];
 	bool destroy_on_close = info->attrs[HWSIM_ATTR_DESTROY_RADIO_ON_CLOSE];
 	bool use_chanctx;
+	char *hwname = NULL;
 
 	if (info->attrs[HWSIM_ATTR_CHANNELS])
 		chans = nla_get_u32(info->attrs[HWSIM_ATTR_CHANNELS]);
 
+	if (info->attrs[HWSIM_ATTR_RADIO_NAME])
+		hwname = nla_data(info->attrs[HWSIM_ATTR_RADIO_NAME]);
+
 	if (info->attrs[HWSIM_ATTR_USE_CHANCTX])
 		use_chanctx = true;
 	else
@@ -2538,7 +2542,8 @@ static int hwsim_create_radio_nl(struct sk_buff *msg, struct genl_info *info)
 
 	return mac80211_hwsim_create_radio(chans, alpha2, regd, reg_strict,
 					   p2p_device, use_chanctx,
-					   destroy_on_close, info->snd_portid);
+					   destroy_on_close, info->snd_portid,
+					   hwname);
 }
 
 static int hwsim_destroy_radio_nl(struct sk_buff *msg, struct genl_info *info)
@@ -2795,7 +2800,7 @@ static int __init init_mac80211_hwsim(void)
 		err = mac80211_hwsim_create_radio(channels, reg_alpha2,
 						  regd, reg_strict,
 						  support_p2p_device,
-						  channels > 1, false, 0);
+						  channels > 1, false, 0, NULL);
 		if (err < 0)
 			goto out_free_radios;
 	}
-- 
1.7.11.7

[PATCH v2 4/6] cfg80211: support creating wiphy w/out creating wlanX.

[greearb]

From: Ben Greear <greearb@candelatech.com>

This will be helpful when using the mac80211_hwsim
wiphys and automated testing.  Let user create the
wlan devs as needed, and named as expected.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 include/net/mac80211.h | 7 ++++++-
 net/mac80211/main.c    | 3 ++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index a206a56..a5236d2 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1579,6 +1579,10 @@ struct ieee80211_tx_control {
  *	a virtual monitor interface when monitor interfaces are the only
  *	active interfaces.
  *
+ * @IEEE80211_HW_NO_AUTO_VDEV: The driver would like for no wlanX to
+ *	be created.  It is expected user-space will create vdevs as
+ *      desired (and thus have them named as desired).
+ *
  * @IEEE80211_HW_QUEUE_CONTROL: The driver wants to control per-interface
  *	queue mapping in order to use different queues (not just one per AC)
  *	for different virtual interfaces. See the doc section on HW queue
@@ -1625,7 +1629,8 @@ enum ieee80211_hw_flags {
 	IEEE80211_HW_SUPPORTS_DYNAMIC_PS		= 1<<12,
 	IEEE80211_HW_MFP_CAPABLE			= 1<<13,
 	IEEE80211_HW_WANT_MONITOR_VIF			= 1<<14,
-	/* free slots */
+	IEEE80211_HW_NO_AUTO_VDEV			= 1<<15,
+	/* free slot */
 	IEEE80211_HW_SUPPORTS_UAPSD			= 1<<17,
 	IEEE80211_HW_REPORTS_TX_ACK_STATUS		= 1<<18,
 	IEEE80211_HW_CONNECTION_MONITOR			= 1<<19,
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index b189122..830bfec 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -1023,7 +1023,8 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
 	}
 
 	/* add one default STA interface if supported */
-	if (local->hw.wiphy->interface_modes & BIT(NL80211_IFTYPE_STATION)) {
+	if (local->hw.wiphy->interface_modes & BIT(NL80211_IFTYPE_STATION) &&
+	    !(hw->flags & IEEE80211_HW_NO_AUTO_VDEV)) {
 		result = ieee80211_if_add(local, "wlan%d", NULL,
 					  NL80211_IFTYPE_STATION, NULL);
 		if (result)
-- 
1.7.11.7

[PATCH v2 5/6] mac80211-hwsim: support creating wiphy w/out creating wlanX.

[greearb]

From: Ben Greear <greearb@candelatech.com>

Good for automated testing, where user can create wlan
interfaces with specified names.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 drivers/net/wireless/mac80211_hwsim.c | 13 ++++++++++---
 drivers/net/wireless/mac80211_hwsim.h |  2 ++
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index ef793d5..fc5695d 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -2023,7 +2023,7 @@ static int mac80211_hwsim_create_radio(int channels, const char *reg_alpha2,
 				       const struct ieee80211_regdomain *regd,
 				       bool reg_strict, bool p2p_device,
 				       bool use_chanctx, bool destroy_on_close,
-				       u32 portid, char *hwname)
+				       u32 portid, char *hwname, bool no_vdev)
 {
 	int err;
 	u8 addr[ETH_ALEN];
@@ -2230,6 +2230,9 @@ static int mac80211_hwsim_create_radio(int channels, const char *reg_alpha2,
 		schedule_timeout_interruptible(1);
 	}
 
+	if (no_vdev)
+		hw->flags |= IEEE80211_HW_NO_AUTO_VDEV;
+
 	err = ieee80211_register_hw(hw);
 	if (err < 0) {
 		printk(KERN_DEBUG "mac80211_hwsim: ieee80211_register_hw failed (%d)\n",
@@ -2516,11 +2519,15 @@ static int hwsim_create_radio_nl(struct sk_buff *msg, struct genl_info *info)
 	bool p2p_device = info->attrs[HWSIM_ATTR_SUPPORT_P2P_DEVICE];
 	bool destroy_on_close = info->attrs[HWSIM_ATTR_DESTROY_RADIO_ON_CLOSE];
 	bool use_chanctx;
+	bool no_vdev = false;
 	char *hwname = NULL;
 
 	if (info->attrs[HWSIM_ATTR_CHANNELS])
 		chans = nla_get_u32(info->attrs[HWSIM_ATTR_CHANNELS]);
 
+	if (info->attrs[HWSIM_ATTR_NO_VDEV])
+		no_vdev = true;
+
 	if (info->attrs[HWSIM_ATTR_RADIO_NAME])
 		hwname = nla_data(info->attrs[HWSIM_ATTR_RADIO_NAME]);
 
@@ -2543,7 +2550,7 @@ static int hwsim_create_radio_nl(struct sk_buff *msg, struct genl_info *info)
 	return mac80211_hwsim_create_radio(chans, alpha2, regd, reg_strict,
 					   p2p_device, use_chanctx,
 					   destroy_on_close, info->snd_portid,
-					   hwname);
+					   hwname, no_vdev);
 }
 
 static int hwsim_destroy_radio_nl(struct sk_buff *msg, struct genl_info *info)
@@ -2800,7 +2807,7 @@ static int __init init_mac80211_hwsim(void)
 		err = mac80211_hwsim_create_radio(channels, reg_alpha2,
 						  regd, reg_strict,
 						  support_p2p_device,
-						  channels > 1, false, 0, NULL);
+						  channels > 1, false, 0, NULL, false);
 		if (err < 0)
 			goto out_free_radios;
 	}
diff --git a/drivers/net/wireless/mac80211_hwsim.h b/drivers/net/wireless/mac80211_hwsim.h
index 98c6998..71bf498 100644
--- a/drivers/net/wireless/mac80211_hwsim.h
+++ b/drivers/net/wireless/mac80211_hwsim.h
@@ -114,6 +114,7 @@ enum {
  * @HWSIM_ATTR_DESTROY_RADIO_ON_CLOSE: used with the %HWSIM_CMD_CREATE_RADIO
  *	command to force radio removal when process that created the radio dies
  * @HWSIM_ATTR_RADIO_NAME: Name of radio, e.g. phy666
+ * @HWSIM_ATTR_NO_VDEV:  Do not create vdev (wlanX) when creating radio.
  * @__HWSIM_ATTR_MAX: enum limit
  */
 
@@ -137,6 +138,7 @@ enum {
 	HWSIM_ATTR_USE_CHANCTX,
 	HWSIM_ATTR_DESTROY_RADIO_ON_CLOSE,
 	HWSIM_ATTR_RADIO_NAME,
+	HWSIM_ATTR_NO_VDEV,
 	__HWSIM_ATTR_MAX,
 };
 #define HWSIM_ATTR_MAX (__HWSIM_ATTR_MAX - 1)
-- 
1.7.11.7

[PATCH v2 6/6] cfg80211: support configuring vdev mac addr on create.

[greearb]

From: Ben Greear <greearb@candelatech.com>

This is useful when creating virtual-stations, and probably
APs and other devices as well.  Keeps udev from mucking with
things it shouldn't, since the default MAC is never seen
by udev when specified on the cmd-line during creation.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 include/net/cfg80211.h       | 6 +++---
 include/uapi/linux/nl80211.h | 3 +++
 net/mac80211/iface.c         | 5 ++++-
 net/mac80211/main.c          | 1 +
 net/wireless/nl80211.c       | 2 +-
 5 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 5252f80..ba8a196 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -319,9 +319,9 @@ struct ieee80211_supported_band {
 /**
  * struct vif_params - describes virtual interface parameters
  * @use_4addr: use 4-address frames
- * @macaddr: address to use for this virtual interface. This will only
- * 	be used for non-netdevice interfaces. If this parameter is set
- * 	to zero address the driver may determine the address as needed.
+ * @macaddr: address to use for this virtual interface.
+ *	If this parameter is set to zero address the driver may
+ *	determine the address as needed.
  */
 struct vif_params {
        int use_4addr;
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index b553c48..c7e35ae 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -4052,6 +4052,8 @@ enum nl80211_ap_sme_features {
  *	multiplexing powersave, ie. can turn off all but one chain
  *	and then wake the rest up as required after, for example,
  *	rts/cts handshake.
+ * @NL80211_FEATURE_MAC_ON_CREATE: Device supports configuring
+ *	the vdev's MAC address upon creation.
  */
 enum nl80211_feature_flags {
 	NL80211_FEATURE_SK_TX_STATUS			= 1 << 0,
@@ -4080,6 +4082,7 @@ enum nl80211_feature_flags {
 	NL80211_FEATURE_ACKTO_ESTIMATION		= 1 << 23,
 	NL80211_FEATURE_STATIC_SMPS			= 1 << 24,
 	NL80211_FEATURE_DYNAMIC_SMPS			= 1 << 25,
+	NL80211_FEATURE_MAC_ON_CREATE			= 1 << 26,
 };
 
 /**
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index e469b33..1ffcc07 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1671,7 +1671,10 @@ int ieee80211_if_add(struct ieee80211_local *local, const char *name,
 		}
 
 		ieee80211_assign_perm_addr(local, ndev->perm_addr, type);
-		memcpy(ndev->dev_addr, ndev->perm_addr, ETH_ALEN);
+		if (params && is_valid_ether_addr(params->macaddr))
+			memcpy(ndev->dev_addr, params->macaddr, ETH_ALEN);
+		else
+			memcpy(ndev->dev_addr, ndev->perm_addr, ETH_ALEN);
 		SET_NETDEV_DEV(ndev, wiphy_dev(local->hw.wiphy));
 
 		/* don't use IEEE80211_DEV_TO_SUB_IF -- it checks too much */
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 830bfec..43d57db 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -542,6 +542,7 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,
 			   NL80211_FEATURE_SAE |
 			   NL80211_FEATURE_HT_IBSS |
 			   NL80211_FEATURE_VIF_TXPOWER |
+			   NL80211_FEATURE_MAC_ON_CREATE |
 			   NL80211_FEATURE_USERSPACE_MPM;
 
 	if (!ops->hw_scan)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 0c0f204..9cbc277 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2605,7 +2605,7 @@ static int nl80211_new_interface(struct sk_buff *skb, struct genl_info *info)
 	    !(rdev->wiphy.interface_modes & (1 << type)))
 		return -EOPNOTSUPP;
 
-	if (type == NL80211_IFTYPE_P2P_DEVICE && info->attrs[NL80211_ATTR_MAC]) {
+	if (info->attrs[NL80211_ATTR_MAC]) {
 		nla_memcpy(params.macaddr, info->attrs[NL80211_ATTR_MAC],
 			   ETH_ALEN);
 		if (!is_valid_ether_addr(params.macaddr))
-- 
1.7.11.7

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

[Johannes Berg]

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

Re: [PATCH v2 2/6] mac80211: allow creating wiphy devices with suggested name.

[Johannes Berg]

There seem to be two spaces in the subject - please fix that since we
need another spin anyway.

> + * @requested_name:  Requested name for this device.

please also just have one space here
also same comments as with cfg80211 - document NULL and the function
name change

> +struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,
> +					   const struct ieee80211_ops *ops,
> +					   const char *requested_name);
> +
> +/**
> + * ieee80211_alloc_hw -  Allocate a new hardware device

another place where you have two spaces :)

johannes

Re: [PATCH v2 3/6] mac80211-hwsim: support creating radios with specific name.

[Johannes Berg]

On Mon, 2014-10-20 at 15:49 -0700, greearb@candelatech.com wrote:

> @@ -2023,7 +2023,7 @@ static int mac80211_hwsim_create_radio(int channels, const char *reg_alpha2,
>  				       const struct ieee80211_regdomain *regd,
>  				       bool reg_strict, bool p2p_device,
>  				       bool use_chanctx, bool destroy_on_close,
> -				       u32 portid)
> +				       u32 portid, char *hwname)

could also be const?

> +	char *hwname = NULL;

const as well then, I guess.

johannes

Re: [PATCH v2 4/6] cfg80211: support creating wiphy w/out creating wlanX.

[Johannes Berg]

This isn't a cfg80211 patch (subject)

> + * @IEEE80211_HW_NO_AUTO_VDEV: The driver would like for no wlanX to
> + *	be created.  It is expected user-space will create vdevs as
> + *      desired (and thus have them named as desired).

"VDEV" isn't a term mac80211 uses anywhere, please use "VIF" or similar
- no need to add more terminology.
johannes

Re: [PATCH v2 5/6] mac80211-hwsim: support creating wiphy w/out creating wlanX.

[Johannes Berg]

On Mon, 2014-10-20 at 15:49 -0700, greearb@candelatech.com wrote:

> +				       u32 portid, char *hwname, bool no_vdev)

same comment regarding "vdev" throughout this patch.

Also, there shouldn't be a . at the end of the subject :-)

johannes

Re: [PATCH v2 6/6] cfg80211: support configuring vdev mac addr on create.

[Johannes Berg]

Same comment regarding "vdev", here starting from the subject.

Also, this patch isn't purely cfg80211 - should probably split it into
cfg80211 and mac80211 parts?

On Mon, 2014-10-20 at 15:49 -0700, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> This is useful when creating virtual-stations, and probably
> APs and other devices as well.

It clearly is - imho worth rephrasing as just "when creating virtual
interfaces" :-)

> +++ b/include/net/cfg80211.h
> @@ -319,9 +319,9 @@ struct ieee80211_supported_band {
>  /**
>   * struct vif_params - describes virtual interface parameters
>   * @use_4addr: use 4-address frames
> - * @macaddr: address to use for this virtual interface. This will only
> - * 	be used for non-netdevice interfaces. If this parameter is set
> - * 	to zero address the driver may determine the address as needed.
> + * @macaddr: address to use for this virtual interface.
> + *	If this parameter is set to zero address the driver may
> + *	determine the address as needed.


Maybe point to the feature flag here.

> + * @NL80211_FEATURE_MAC_ON_CREATE: Device supports configuring
> + *	the vdev's MAC address upon creation.

Maybe point to the attribute that's used for that here?

johannes

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

[Ben Greear]

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 <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

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

[Ben Greear]

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
> 


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