All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v9 0/5] cfg80211/mac80211: move interface combinations check to mac80211
@ 2014-03-10 21:31 Luciano Coelho
  2014-03-10 21:31 ` [PATCH v9 1/5] cfg80211: refactor cfg80211_can_use_iftype_chan() Luciano Coelho
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Luciano Coelho @ 2014-03-10 21:31 UTC (permalink / raw)
  To: linux-wireless; +Cc: johannes, michal.kazior, sw, andrei.otcheretianski

Hi,

Addressed some offline comments by Johannes,

In v9:

   * pass sdata->wdev.iftype instead of sdata->vif.type when calling
     cfg80211_chandef_dfs_required();
   * fixed commit message in 2/5;
   * added braces in a couple of if's that had comments and a
     statement;
   * move NL80211_IFTYPE_UNSPECIFIED to the WARN part of the switch in
     dfs_required) and remove default;

In v8:

   * fixed rebasing mistakes when setting radar_detect_width in
     patches 3/5 and 4/5;

In v7:

   * added a new patch to get rid of some more combination checks in
     cfg80211 (Johannes);
   * remove local argument from ieee80211_check_combinations();
   * remove redundant arguments from cfg80211_check_combinations();
     (Johannes);
   * change cfg80211_chandef_dfs_required() back to returning only 0
     or 1 instead of the channel width bitmap (Johannes);
   * remove a useless TODO in the cfg80211 doc (Johannes);

In v6:

   * fix typo in the interface counting code
     s/sdata-vif.chanctx_conf/sdata_iter-vif.chanctx_conf/

In v5:

   * use local and sdata in ibss when calling
     ieee80211_check_combinations();
   * use the radar detection widths from all existing contexts,
     regardless of whether we can share an existing context or if the
     context is exclusive (thanks Michal!);

In v4:

   * rebased the remaining patches on top of what Johannes applied;
   * lots of other small changes in response to Johannes's comments.
     See each individual patch for detailed lists of changed (thanks
     Johannes!);

In v3:

   * move the "pass the mode argument..." change to the right patch;

In v2:

   * lock the chanctx mutex in ieee80211_ibss_join() before calling
     ieee80211_check_combinations().

   * pass the mode argument instead of IEEE80211_CHANCTX_SHARED to
     ieee80211_check_combinations() in ieee80211_vif_use_channel();

As Johannes suggested earlier, this patchset moves the interface
combinations check from cfg80211 to mac80211.  This is needed to
simplify the channel switch code.

There are still some cases where cfg80211 may need to check the
interface combinations (ie. when adding or changing an interface), so
I created a helper function that can be called by cfg80211 itself or
the drivers (such as mac80211).

Also, for now, I only moved the check in the cases where
ieee80211_vif_use_channel() is called and for IBSS join.  The channel
switch stuff, which will be trickier, is still missing.  These will be
handled separately and it doesn't really hurt to leave it as it is,
except because it's a bit inconsistent.

I had to shuffle some code around to make this whole thing a bit
cleaner, including the check for whether radar detection is needed.

This is not very thoroughly tested yet, I have just done some checks
with AP and STA with and without chanctx and with 1 or 2 different
channels.  This part seems to work, please test the rest. :)

I'll try to add some hwsim tests to test this more thoroughly.

Luca.


Luciano Coelho (5):
  cfg80211: refactor cfg80211_can_use_iftype_chan()
  cfg80211/mac80211: refactor cfg80211_chandef_dfs_required()
  cfg80211/mac80211: move interface counting for combination check to
    mac80211
  cfg80211/mac80211: move combination check to mac80211 for ibss
  cfg80211/mac80211: move more combination checks to mac80211

 Documentation/DocBook/80211.tmpl |   1 +
 include/net/cfg80211.h           |  31 +++++++--
 net/mac80211/cfg.c               |  11 +++-
 net/mac80211/chan.c              |  17 +++++
 net/mac80211/ibss.c              |  64 ++++++++++++------
 net/mac80211/ieee80211_i.h       |   4 ++
 net/mac80211/iface.c             |   6 +-
 net/mac80211/mesh.c              |   9 +--
 net/mac80211/util.c              |  76 +++++++++++++++++++++
 net/wireless/chan.c              |  74 ++++++++++++++++-----
 net/wireless/core.c              |  11 +---
 net/wireless/core.h              |  29 --------
 net/wireless/ibss.c              |  24 -------
 net/wireless/mesh.c              |  25 -------
 net/wireless/mlme.c              |  14 +---
 net/wireless/nl80211.c           |  59 +++++------------
 net/wireless/util.c              | 138 ++++++++++++++++++++++-----------------
 17 files changed, 343 insertions(+), 250 deletions(-)

-- 
1.9.0


^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH v9 1/5] cfg80211: refactor cfg80211_can_use_iftype_chan()
  2014-03-10 21:31 [PATCH v9 0/5] cfg80211/mac80211: move interface combinations check to mac80211 Luciano Coelho
@ 2014-03-10 21:31 ` Luciano Coelho
  2014-03-11 10:40   ` Eliad Peller
  2014-03-10 21:31 ` [PATCH v9 2/5] cfg80211/mac80211: refactor cfg80211_chandef_dfs_required() Luciano Coelho
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Luciano Coelho @ 2014-03-10 21:31 UTC (permalink / raw)
  To: linux-wireless; +Cc: johannes, michal.kazior, sw, andrei.otcheretianski

Separate the code that counts the interface types and channels from
the code that check the interface combinations.  The new function that
checks for combinations is exported so it can be called by the
drivers.

This is done in preparation for moving the interface combinations
checks out of cfg80211.

Signed-off-by: Luciano Coelho <luciano.coelho@intel.com>
---
In v4:

   * rebased on top of slightly modified applied patches
   * removed the parenthesis from the comment (not needed for docbook)
   * use num[NUM_NL80211_IFTYPES] in cfg80211_check_combinations()

In v7:

   * removed redundant arguments (num_interfaces and used_iftypes)
     from cfg80211_check_combinations();
---
 Documentation/DocBook/80211.tmpl |   1 +
 include/net/cfg80211.h           |  22 +++++++
 net/wireless/util.c              | 128 ++++++++++++++++++++++-----------------
 3 files changed, 95 insertions(+), 56 deletions(-)

diff --git a/Documentation/DocBook/80211.tmpl b/Documentation/DocBook/80211.tmpl
index 044b764..d9b9416 100644
--- a/Documentation/DocBook/80211.tmpl
+++ b/Documentation/DocBook/80211.tmpl
@@ -100,6 +100,7 @@
 !Finclude/net/cfg80211.h wdev_priv
 !Finclude/net/cfg80211.h ieee80211_iface_limit
 !Finclude/net/cfg80211.h ieee80211_iface_combination
+!Finclude/net/cfg80211.h cfg80211_check_combinations
       </chapter>
       <chapter>
       <title>Actions and configuration</title>
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index ff3af16..768fcc0 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -4682,6 +4682,28 @@ void cfg80211_crit_proto_stopped(struct wireless_dev *wdev, gfp_t gfp);
  */
 unsigned int ieee80211_get_num_supported_channels(struct wiphy *wiphy);
 
+/**
+ * cfg80211_check_combinations - check interface combinations
+ *
+ * @wiphy: the wiphy
+ * @num_different_channels: the number of different channels we want
+ *	to use for verification
+ * @radar_detect: a bitmap where each bit corresponds to a channel
+ *	width where radar detection is needed, as in the definition of
+ *	&struct ieee80211_iface_combination.@radar_detect_widths
+ * @iftype_num: array with the numbers of interfaces of each interface
+ *	type.  The index is the interface type as specified in &enum
+ *	nl80211_iftype.
+ *
+ * This function can be called by the driver to check whether a
+ * combination of interfaces and their types are allowed according to
+ * the interface combinations.
+ */
+int cfg80211_check_combinations(struct wiphy *wiphy,
+				const int num_different_channels,
+				const u8 radar_detect,
+				const int iftype_num[NUM_NL80211_IFTYPES]);
+
 /* Logging, debugging and troubleshooting/diagnostic helpers. */
 
 /* wiphy_printk helpers, similar to dev_printk */
diff --git a/net/wireless/util.c b/net/wireless/util.c
index dadc934..7ecc0ef 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -1253,6 +1253,75 @@ int cfg80211_validate_beacon_int(struct cfg80211_registered_device *rdev,
 	return res;
 }
 
+int cfg80211_check_combinations(struct wiphy *wiphy,
+				const int num_different_channels,
+				const u8 radar_detect,
+				const int iftype_num[NUM_NL80211_IFTYPES])
+{
+	int i, j, iftype;
+	int num_interfaces = 0;
+	u32 used_iftypes = 0;
+
+	for (iftype = 0; iftype < NUM_NL80211_IFTYPES; iftype++) {
+		num_interfaces += iftype_num[iftype];
+		if (iftype_num[iftype] > 0)
+			used_iftypes |= BIT(iftype);
+	}
+
+	for (i = 0; i < wiphy->n_iface_combinations; i++) {
+		const struct ieee80211_iface_combination *c;
+		struct ieee80211_iface_limit *limits;
+		u32 all_iftypes = 0;
+
+		c = &wiphy->iface_combinations[i];
+
+		if (num_interfaces > c->max_interfaces)
+			continue;
+		if (num_different_channels > c->num_different_channels)
+			continue;
+
+		limits = kmemdup(c->limits, sizeof(limits[0]) * c->n_limits,
+				 GFP_KERNEL);
+		if (!limits)
+			return -ENOMEM;
+
+		for (iftype = 0; iftype < NUM_NL80211_IFTYPES; iftype++) {
+			if (wiphy->software_iftypes & BIT(iftype))
+				continue;
+			for (j = 0; j < c->n_limits; j++) {
+				all_iftypes |= limits[j].types;
+				if (!(limits[j].types & BIT(iftype)))
+					continue;
+				if (limits[j].max < iftype_num[iftype])
+					goto cont;
+				limits[j].max -= iftype_num[iftype];
+			}
+		}
+
+		if (radar_detect && !(c->radar_detect_widths & radar_detect))
+			goto cont;
+
+		/* Finally check that all iftypes that we're currently
+		 * using are actually part of this combination. If they
+		 * aren't then we can't use this combination and have
+		 * to continue to the next.
+		 */
+		if ((all_iftypes & used_iftypes) != used_iftypes)
+			goto cont;
+
+		/* This combination covered all interface types and
+		 * supported the requested numbers, so we're good.
+		 */
+		kfree(limits);
+		return 0;
+ cont:
+		kfree(limits);
+	}
+
+	return -EBUSY;
+}
+EXPORT_SYMBOL(cfg80211_check_combinations);
+
 int cfg80211_can_use_iftype_chan(struct cfg80211_registered_device *rdev,
 				 struct wireless_dev *wdev,
 				 enum nl80211_iftype iftype,
@@ -1261,7 +1330,6 @@ int cfg80211_can_use_iftype_chan(struct cfg80211_registered_device *rdev,
 				 u8 radar_detect)
 {
 	struct wireless_dev *wdev_iter;
-	u32 used_iftypes = BIT(iftype);
 	int num[NUM_NL80211_IFTYPES];
 	struct ieee80211_channel
 			*used_channels[CFG80211_MAX_NUM_DIFFERENT_CHANNELS];
@@ -1269,7 +1337,7 @@ int cfg80211_can_use_iftype_chan(struct cfg80211_registered_device *rdev,
 	enum cfg80211_chan_mode chmode;
 	int num_different_channels = 0;
 	int total = 1;
-	int i, j;
+	int i;
 
 	ASSERT_RTNL();
 
@@ -1354,65 +1422,13 @@ int cfg80211_can_use_iftype_chan(struct cfg80211_registered_device *rdev,
 
 		num[wdev_iter->iftype]++;
 		total++;
-		used_iftypes |= BIT(wdev_iter->iftype);
 	}
 
 	if (total == 1 && !radar_detect)
 		return 0;
 
-	for (i = 0; i < rdev->wiphy.n_iface_combinations; i++) {
-		const struct ieee80211_iface_combination *c;
-		struct ieee80211_iface_limit *limits;
-		u32 all_iftypes = 0;
-
-		c = &rdev->wiphy.iface_combinations[i];
-
-		if (total > c->max_interfaces)
-			continue;
-		if (num_different_channels > c->num_different_channels)
-			continue;
-
-		limits = kmemdup(c->limits, sizeof(limits[0]) * c->n_limits,
-				 GFP_KERNEL);
-		if (!limits)
-			return -ENOMEM;
-
-		for (iftype = 0; iftype < NUM_NL80211_IFTYPES; iftype++) {
-			if (rdev->wiphy.software_iftypes & BIT(iftype))
-				continue;
-			for (j = 0; j < c->n_limits; j++) {
-				all_iftypes |= limits[j].types;
-				if (!(limits[j].types & BIT(iftype)))
-					continue;
-				if (limits[j].max < num[iftype])
-					goto cont;
-				limits[j].max -= num[iftype];
-			}
-		}
-
-		if (radar_detect && !(c->radar_detect_widths & radar_detect))
-			goto cont;
-
-		/*
-		 * Finally check that all iftypes that we're currently
-		 * using are actually part of this combination. If they
-		 * aren't then we can't use this combination and have
-		 * to continue to the next.
-		 */
-		if ((all_iftypes & used_iftypes) != used_iftypes)
-			goto cont;
-
-		/*
-		 * This combination covered all interface types and
-		 * supported the requested numbers, so we're good.
-		 */
-		kfree(limits);
-		return 0;
- cont:
-		kfree(limits);
-	}
-
-	return -EBUSY;
+	return cfg80211_check_combinations(&rdev->wiphy, num_different_channels,
+					   radar_detect, num);
 }
 
 int ieee80211_get_ratemask(struct ieee80211_supported_band *sband,
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH v9 2/5] cfg80211/mac80211: refactor cfg80211_chandef_dfs_required()
  2014-03-10 21:31 [PATCH v9 0/5] cfg80211/mac80211: move interface combinations check to mac80211 Luciano Coelho
  2014-03-10 21:31 ` [PATCH v9 1/5] cfg80211: refactor cfg80211_can_use_iftype_chan() Luciano Coelho
@ 2014-03-10 21:31 ` Luciano Coelho
  2014-03-11 10:45   ` Eliad Peller
  2014-03-10 21:31 ` [PATCH v9 3/5] cfg80211/mac80211: move interface counting for combination check to mac80211 Luciano Coelho
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Luciano Coelho @ 2014-03-10 21:31 UTC (permalink / raw)
  To: linux-wireless; +Cc: johannes, michal.kazior, sw, andrei.otcheretianski

Some interface types don't require DFS (such as STATION, P2P_CLIENT
etc).  In order to centralize these decisions, make
cfg80211_chandef_dfs_required() take the iftype into consideration.

Signed-off-by: Luciano Coelho <luciano.coelho@intel.com>
---
In v4:

   * rebased on top of slightly modified applied patches
   * removed the double negations when assigning err to a boolean;

In v7:

   * change cfg80211_chandef_dfs_required() back to returning 1 if
     radar detection is needed instead of a bitmap with the width
     required.

In v9:

   * fixed commit message;
   * added braces in a couple of if's that had comments and a
     statement;
   * move NL80211_IFTYPE_UNSPECIFIED to the WARN part of the switch in
     dfs_required) and remove default;
---
 include/net/cfg80211.h |  7 +++--
 net/mac80211/ibss.c    | 32 +++++++++++-----------
 net/mac80211/mesh.c    |  9 +++---
 net/wireless/chan.c    | 74 ++++++++++++++++++++++++++++++++++++++------------
 net/wireless/mesh.c    |  7 +++--
 net/wireless/nl80211.c | 37 ++++++++++++-------------
 6 files changed, 104 insertions(+), 62 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 768fcc0..51f278a 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -441,10 +441,13 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy,
  * cfg80211_chandef_dfs_required - checks if radar detection is required
  * @wiphy: the wiphy to validate against
  * @chandef: the channel definition to check
- * Return: 1 if radar detection is required, 0 if it is not, < 0 on error
+ * @iftype: the interface type as specified in &enum nl80211_iftype
+ * Returns:
+ *	1 if radar detection is required, 0 if it is not, < 0 on error
  */
 int cfg80211_chandef_dfs_required(struct wiphy *wiphy,
-				  const struct cfg80211_chan_def *chandef);
+				  const struct cfg80211_chan_def *chandef,
+				  enum nl80211_iftype);
 
 /**
  * ieee80211_chandef_rate_flags - returns rate flags for a channel
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index e458ca0..12057fd 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -228,7 +228,7 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 	struct beacon_data *presp;
 	enum nl80211_bss_scan_width scan_width;
 	bool have_higher_than_11mbit;
-	bool radar_required = false;
+	bool radar_required;
 	int err;
 
 	sdata_assert_lock(sdata);
@@ -282,21 +282,20 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 	}
 
 	err = cfg80211_chandef_dfs_required(sdata->local->hw.wiphy,
-					    &chandef);
+					    &chandef, NL80211_IFTYPE_ADHOC);
 	if (err < 0) {
 		sdata_info(sdata,
 			   "Failed to join IBSS, invalid chandef\n");
 		return;
 	}
-	if (err > 0) {
-		if (!ifibss->userspace_handles_dfs) {
-			sdata_info(sdata,
-				   "Failed to join IBSS, DFS channel without control program\n");
-			return;
-		}
-		radar_required = true;
+	if (err > 0 && !ifibss->userspace_handles_dfs) {
+		sdata_info(sdata,
+			   "Failed to join IBSS, DFS channel without control program\n");
+		return;
 	}
 
+	radar_required = err;
+
 	mutex_lock(&local->mtx);
 	if (ieee80211_vif_use_channel(sdata, &chandef,
 				      ifibss->fixed_channel ?
@@ -775,7 +774,8 @@ static void ieee80211_ibss_csa_mark_radar(struct ieee80211_sub_if_data *sdata)
 	 * unavailable.
 	 */
 	err = cfg80211_chandef_dfs_required(sdata->local->hw.wiphy,
-					    &ifibss->chandef);
+					    &ifibss->chandef,
+					    NL80211_IFTYPE_ADHOC);
 	if (err > 0)
 		cfg80211_radar_event(sdata->local->hw.wiphy, &ifibss->chandef,
 				     GFP_ATOMIC);
@@ -873,17 +873,17 @@ ieee80211_ibss_process_chanswitch(struct ieee80211_sub_if_data *sdata,
 	}
 
 	err = cfg80211_chandef_dfs_required(sdata->local->hw.wiphy,
-					    &params.chandef);
+					    &params.chandef,
+					    NL80211_IFTYPE_ADHOC);
 	if (err < 0)
 		goto disconnect;
-	if (err) {
+	if (err > 0 && !ifibss->userspace_handles_dfs) {
 		/* IBSS-DFS only allowed with a control program */
-		if (!ifibss->userspace_handles_dfs)
-			goto disconnect;
-
-		params.radar_required = true;
+		goto disconnect;
 	}
 
+	params.radar_required = err;
+
 	if (cfg80211_chandef_identical(&params.chandef,
 				       &sdata->vif.bss_conf.chandef)) {
 		ibss_dbg(sdata,
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index f70e9cd..ea0b628 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -903,14 +903,15 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
 	}
 
 	err = cfg80211_chandef_dfs_required(sdata->local->hw.wiphy,
-					    &params.chandef);
+					    &params.chandef,
+					    NL80211_IFTYPE_MESH_POINT);
 	if (err < 0)
 		return false;
-	if (err) {
-		params.radar_required = true;
+	if (err > 0)
 		/* TODO: DFS not (yet) supported */
 		return false;
-	}
+
+	params.radar_required = err;
 
 	if (cfg80211_chandef_identical(&params.chandef,
 				       &sdata->vif.bss_conf.chandef)) {
diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index 8659d5c..936e58a 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -326,28 +326,57 @@ static int cfg80211_get_chans_dfs_required(struct wiphy *wiphy,
 
 
 int cfg80211_chandef_dfs_required(struct wiphy *wiphy,
-				  const struct cfg80211_chan_def *chandef)
+				  const struct cfg80211_chan_def *chandef,
+				  enum nl80211_iftype iftype)
 {
 	int width;
-	int r;
+	int ret;
 
 	if (WARN_ON(!cfg80211_chandef_valid(chandef)))
 		return -EINVAL;
 
-	width = cfg80211_chandef_get_width(chandef);
-	if (width < 0)
-		return -EINVAL;
+	switch (iftype) {
+	case NL80211_IFTYPE_ADHOC:
+	case NL80211_IFTYPE_AP:
+	case NL80211_IFTYPE_P2P_GO:
+	case NL80211_IFTYPE_MESH_POINT:
+		width = cfg80211_chandef_get_width(chandef);
+		if (width < 0)
+			return -EINVAL;
 
-	r = cfg80211_get_chans_dfs_required(wiphy, chandef->center_freq1,
-					    width);
-	if (r)
-		return r;
+		ret = cfg80211_get_chans_dfs_required(wiphy,
+						      chandef->center_freq1,
+						      width);
+		if (ret < 0)
+			return ret;
+		else if (ret > 0)
+			return BIT(chandef->width);
 
-	if (!chandef->center_freq2)
-		return 0;
+		if (!chandef->center_freq2)
+			return 0;
+
+		ret = cfg80211_get_chans_dfs_required(wiphy,
+						      chandef->center_freq2,
+						      width);
+		if (ret < 0)
+			return ret;
+		else if (ret > 0)
+			return BIT(chandef->width);
 
-	return cfg80211_get_chans_dfs_required(wiphy, chandef->center_freq2,
-					       width);
+		break;
+	case NL80211_IFTYPE_STATION:
+	case NL80211_IFTYPE_P2P_CLIENT:
+	case NL80211_IFTYPE_MONITOR:
+	case NL80211_IFTYPE_AP_VLAN:
+	case NL80211_IFTYPE_WDS:
+	case NL80211_IFTYPE_P2P_DEVICE:
+		break;
+	case NL80211_IFTYPE_UNSPECIFIED:
+	case NUM_NL80211_IFTYPES:
+		WARN_ON(1);
+	}
+
+	return 0;
 }
 EXPORT_SYMBOL(cfg80211_chandef_dfs_required);
 
@@ -671,7 +700,8 @@ bool cfg80211_reg_can_beacon(struct wiphy *wiphy,
 
 	trace_cfg80211_reg_can_beacon(wiphy, chandef);
 
-	if (cfg80211_chandef_dfs_required(wiphy, chandef) > 0 &&
+	if (cfg80211_chandef_dfs_required(wiphy, chandef,
+					  NL80211_IFTYPE_UNSPECIFIED) > 0 &&
 	    cfg80211_chandef_dfs_available(wiphy, chandef)) {
 		/* We can skip IEEE80211_CHAN_NO_IR if chandef dfs available */
 		prohibited_flags = IEEE80211_CHAN_DISABLED;
@@ -701,6 +731,8 @@ cfg80211_get_chan_state(struct wireless_dev *wdev,
 		        enum cfg80211_chan_mode *chanmode,
 		        u8 *radar_detect)
 {
+	int ret;
+
 	*chan = NULL;
 	*chanmode = CHAN_MODE_UNDEFINED;
 
@@ -743,8 +775,11 @@ cfg80211_get_chan_state(struct wireless_dev *wdev,
 			*chan = wdev->chandef.chan;
 			*chanmode = CHAN_MODE_SHARED;
 
-			if (cfg80211_chandef_dfs_required(wdev->wiphy,
-							  &wdev->chandef))
+			ret = cfg80211_chandef_dfs_required(wdev->wiphy,
+							    &wdev->chandef,
+							    wdev->iftype);
+			WARN_ON(ret < 0);
+			if (ret > 0)
 				*radar_detect |= BIT(wdev->chandef.width);
 		}
 		return;
@@ -753,8 +788,11 @@ cfg80211_get_chan_state(struct wireless_dev *wdev,
 			*chan = wdev->chandef.chan;
 			*chanmode = CHAN_MODE_SHARED;
 
-			if (cfg80211_chandef_dfs_required(wdev->wiphy,
-							  &wdev->chandef))
+			ret = cfg80211_chandef_dfs_required(wdev->wiphy,
+							    &wdev->chandef,
+							    wdev->iftype);
+			WARN_ON(ret < 0);
+			if (ret > 0)
 				*radar_detect |= BIT(wdev->chandef.width);
 		}
 		return;
diff --git a/net/wireless/mesh.c b/net/wireless/mesh.c
index 5af5cc6..a95212b 100644
--- a/net/wireless/mesh.c
+++ b/net/wireless/mesh.c
@@ -178,10 +178,13 @@ int __cfg80211_join_mesh(struct cfg80211_registered_device *rdev,
 	if (!cfg80211_reg_can_beacon(&rdev->wiphy, &setup->chandef))
 		return -EINVAL;
 
-	err = cfg80211_chandef_dfs_required(wdev->wiphy, &setup->chandef);
+	err = cfg80211_chandef_dfs_required(wdev->wiphy,
+					    &setup->chandef,
+					    NL80211_IFTYPE_MESH_POINT);
 	if (err < 0)
 		return err;
-	if (err)
+
+	if (err > 0)
 		radar_detect_width = BIT(setup->chandef.width);
 
 	err = cfg80211_can_use_iftype_chan(rdev, wdev, wdev->iftype,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 052c1bf..9fe7746 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -3261,12 +3261,15 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
 	if (!cfg80211_reg_can_beacon(&rdev->wiphy, &params.chandef))
 		return -EINVAL;
 
-	err = cfg80211_chandef_dfs_required(wdev->wiphy, &params.chandef);
+	err = cfg80211_chandef_dfs_required(wdev->wiphy,
+					    &params.chandef,
+					    NL80211_IFTYPE_AP);
 	if (err < 0)
 		return err;
-	if (err) {
-		radar_detect_width = BIT(params.chandef.width);
+
+	if (err > 0) {
 		params.radar_required = true;
+		radar_detect_width = BIT(params.chandef.width);
 	}
 
 	err = cfg80211_can_use_iftype_chan(rdev, wdev, wdev->iftype,
@@ -5796,7 +5799,8 @@ static int nl80211_start_radar_detection(struct sk_buff *skb,
 	if (wdev->cac_started)
 		return -EBUSY;
 
-	err = cfg80211_chandef_dfs_required(wdev->wiphy, &chandef);
+	err = cfg80211_chandef_dfs_required(wdev->wiphy, &chandef,
+					    NL80211_IFTYPE_UNSPECIFIED);
 	if (err < 0)
 		return err;
 
@@ -5931,22 +5935,15 @@ skip_beacons:
 	if (!cfg80211_reg_can_beacon(&rdev->wiphy, &params.chandef))
 		return -EINVAL;
 
-	switch (dev->ieee80211_ptr->iftype) {
-	case NL80211_IFTYPE_AP:
-	case NL80211_IFTYPE_P2P_GO:
-	case NL80211_IFTYPE_ADHOC:
-	case NL80211_IFTYPE_MESH_POINT:
-		err = cfg80211_chandef_dfs_required(wdev->wiphy,
-						    &params.chandef);
-		if (err < 0)
-			return err;
-		if (err) {
-			radar_detect_width = BIT(params.chandef.width);
-			params.radar_required = true;
-		}
-		break;
-	default:
-		break;
+	err = cfg80211_chandef_dfs_required(wdev->wiphy,
+					    &params.chandef,
+					    wdev->iftype);
+	if (err < 0)
+		return err;
+
+	if (err > 0) {
+		radar_detect_width = BIT(params.chandef.width);
+		params.radar_required = true;
 	}
 
 	err = cfg80211_can_use_iftype_chan(rdev, wdev, wdev->iftype,
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH v9 3/5] cfg80211/mac80211: move interface counting for combination check to mac80211
  2014-03-10 21:31 [PATCH v9 0/5] cfg80211/mac80211: move interface combinations check to mac80211 Luciano Coelho
  2014-03-10 21:31 ` [PATCH v9 1/5] cfg80211: refactor cfg80211_can_use_iftype_chan() Luciano Coelho
  2014-03-10 21:31 ` [PATCH v9 2/5] cfg80211/mac80211: refactor cfg80211_chandef_dfs_required() Luciano Coelho
@ 2014-03-10 21:31 ` Luciano Coelho
  2014-03-10 21:31 ` [PATCH v9 4/5] cfg80211/mac80211: move combination check to mac80211 for ibss Luciano Coelho
  2014-03-10 21:31 ` [PATCH v9 5/5] cfg80211/mac80211: move more combination checks to mac80211 Luciano Coelho
  4 siblings, 0 replies; 10+ messages in thread
From: Luciano Coelho @ 2014-03-10 21:31 UTC (permalink / raw)
  To: linux-wireless; +Cc: johannes, michal.kazior, sw, andrei.otcheretianski

Move the counting part of the interface combination check from
cfg80211 to mac80211.

This is needed to simplify locking when the driver has to perform a
combination check by itself (eg. with channel-switch).

Signed-off-by: Luciano Coelho <luciano.coelho@intel.com>
---
In v3:

   * pass the mode argument instead of IEEE80211_CHANCTX_SHARED to
     ieee80211_check_combinations() in ieee80211_vif_use_channel();

In v4:

   * rebased on top of slightly modified applied patches;
   * removed WARN_ON in ieee80211_start_radar_detection();
   * removed unnecessary TODOs in ieee80211_mgd_auth() and
     ieee80211_mgd_assoc();
   * use local and sdata instead of wiphy and wdev in
     ieee80211_check_combinations();
   * check if the vif has a chanctx instead of checking the type and
     if it is running in the interfaces iterator in
     ieee80211_check_combinations();
   * combined the "continue" cases in the iteration into a single if;
   * removed cfg80211_can_use_chan() since libertas mesh, the only
     remaining caller, doesn't really need it.

In v5:

   * use the radar detection widths from all existing contexts,
     regardless of whether we can share an existing context or if the
     context is exclusive.

In v6:

   * fix typo in the interface counting code
     s/sdata-vif.chanctx_conf/sdata_iter-vif.chanctx_conf/

In v7:

   * use new version of cfg80211_check_combinations()
   * remove unnecessary TODO from the documentation
   * remove the local argument in ieee80211_check_combinations, since
     it can be dereferenced from sdata anyway;

In v8:

   * fix radar_detect_width before calling
     ieee80211_check_combinations() -- rebase bug;

In v9:

   * pass sdata->wdev.iftype instead of sdata->vif.type when calling
     cfg80211_chandef_dfs_required();
---
 include/net/cfg80211.h     |  2 --
 net/mac80211/cfg.c         |  2 --
 net/mac80211/chan.c        | 17 +++++++++++
 net/mac80211/ieee80211_i.h |  4 +++
 net/mac80211/util.c        | 72 ++++++++++++++++++++++++++++++++++++++++++++++
 net/wireless/core.h        | 13 ++-------
 net/wireless/ibss.c        |  4 +++
 net/wireless/mesh.c        | 28 ------------------
 net/wireless/mlme.c        | 14 +--------
 net/wireless/nl80211.c     | 29 +++----------------
 net/wireless/util.c        |  5 ++++
 11 files changed, 110 insertions(+), 80 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 51f278a..23a72e2 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -657,7 +657,6 @@ struct cfg80211_acl_data {
  * @p2p_opp_ps: P2P opportunistic PS
  * @acl: ACL configuration used by the drivers which has support for
  *	MAC address based access control
- * @radar_required: set if radar detection is required
  */
 struct cfg80211_ap_settings {
 	struct cfg80211_chan_def chandef;
@@ -675,7 +674,6 @@ struct cfg80211_ap_settings {
 	u8 p2p_ctwindow;
 	bool p2p_opp_ps;
 	const struct cfg80211_acl_data *acl;
-	bool radar_required;
 };
 
 /**
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index aaa59d7..9b555e0 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -972,7 +972,6 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
 	sdata->needed_rx_chains = sdata->local->rx_chains;
 
 	mutex_lock(&local->mtx);
-	sdata->radar_required = params->radar_required;
 	err = ieee80211_vif_use_channel(sdata, &params->chandef,
 					IEEE80211_CHANCTX_SHARED);
 	mutex_unlock(&local->mtx);
@@ -2930,7 +2929,6 @@ static int ieee80211_start_radar_detection(struct wiphy *wiphy,
 	/* whatever, but channel contexts should not complain about that one */
 	sdata->smps_mode = IEEE80211_SMPS_OFF;
 	sdata->needed_rx_chains = local->rx_chains;
-	sdata->radar_required = true;
 
 	err = ieee80211_vif_use_channel(sdata, chandef,
 					IEEE80211_CHANCTX_SHARED);
diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index 42c6592..384f3c7 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -513,6 +513,7 @@ int ieee80211_vif_use_channel(struct ieee80211_sub_if_data *sdata,
 {
 	struct ieee80211_local *local = sdata->local;
 	struct ieee80211_chanctx *ctx;
+	u8 radar_detect_width = 0;
 	int ret;
 
 	lockdep_assert_held(&local->mtx);
@@ -520,6 +521,22 @@ int ieee80211_vif_use_channel(struct ieee80211_sub_if_data *sdata,
 	WARN_ON(sdata->dev && netif_carrier_ok(sdata->dev));
 
 	mutex_lock(&local->chanctx_mtx);
+
+	ret = cfg80211_chandef_dfs_required(local->hw.wiphy,
+					    chandef,
+					    sdata->wdev.iftype);
+	if (ret < 0)
+		goto out;
+	if (ret > 0)
+		radar_detect_width = BIT(chandef->width);
+
+	sdata->radar_required = ret;
+
+	ret = ieee80211_check_combinations(sdata, chandef, mode,
+					   radar_detect_width);
+	if (ret < 0)
+		goto out;
+
 	__ieee80211_vif_release_channel(sdata);
 
 	ctx = ieee80211_find_chanctx(local, chandef, mode);
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 8603dfb..d3ebe7f 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1810,6 +1810,10 @@ int ieee80211_cs_headroom(struct ieee80211_local *local,
 			  enum nl80211_iftype iftype);
 void ieee80211_recalc_dtim(struct ieee80211_local *local,
 			   struct ieee80211_sub_if_data *sdata);
+int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
+				 const struct cfg80211_chan_def *chandef,
+				 enum ieee80211_chanctx_mode chanmode,
+				 u8 radar_detect);
 
 #ifdef CONFIG_MAC80211_NOINLINE
 #define debug_noinline noinline
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index d842af5..3972b64 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -2801,3 +2801,75 @@ void ieee80211_recalc_dtim(struct ieee80211_local *local,
 
 	ps->dtim_count = dtim_count;
 }
+
+int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
+				 const struct cfg80211_chan_def *chandef,
+				 enum ieee80211_chanctx_mode chanmode,
+				 u8 radar_detect)
+{
+	struct ieee80211_local *local = sdata->local;
+	struct ieee80211_sub_if_data *sdata_iter;
+	enum nl80211_iftype iftype = sdata->wdev.iftype;
+	int num[NUM_NL80211_IFTYPES];
+	struct ieee80211_chanctx *ctx;
+	int num_different_channels = 1;
+	int total = 1;
+
+	lockdep_assert_held(&local->chanctx_mtx);
+
+	if (WARN_ON(hweight32(radar_detect) > 1))
+		return -EINVAL;
+
+	if (WARN_ON(chanmode == IEEE80211_CHANCTX_SHARED && !chandef->chan))
+		return -EINVAL;
+
+	if (WARN_ON(iftype >= NUM_NL80211_IFTYPES))
+		return -EINVAL;
+
+	/* Always allow software iftypes */
+	if (local->hw.wiphy->software_iftypes & BIT(iftype)) {
+		if (radar_detect)
+			return -EINVAL;
+		return 0;
+	}
+
+	memset(num, 0, sizeof(num));
+
+	if (iftype != NL80211_IFTYPE_UNSPECIFIED)
+		num[iftype] = 1;
+
+	list_for_each_entry(ctx, &local->chanctx_list, list) {
+		if (ctx->conf.radar_enabled)
+			radar_detect |= BIT(ctx->conf.def.width);
+		if (ctx->mode == IEEE80211_CHANCTX_EXCLUSIVE) {
+			num_different_channels++;
+			continue;
+		}
+		if ((chanmode == IEEE80211_CHANCTX_SHARED) &&
+		    cfg80211_chandef_compatible(chandef,
+						&ctx->conf.def))
+			continue;
+		num_different_channels++;
+	}
+
+	list_for_each_entry_rcu(sdata_iter, &local->interfaces, list) {
+		struct wireless_dev *wdev_iter;
+
+		wdev_iter = &sdata_iter->wdev;
+
+		if (sdata_iter == sdata ||
+		    rcu_access_pointer(sdata_iter->vif.chanctx_conf) == NULL ||
+		    local->hw.wiphy->software_iftypes & BIT(wdev_iter->iftype))
+			continue;
+
+		num[wdev_iter->iftype]++;
+		total++;
+	}
+
+	if (total == 1 && !radar_detect)
+		return 0;
+
+	return cfg80211_check_combinations(local->hw.wiphy,
+					   num_different_channels,
+					   radar_detect, num);
+}
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 3975ffa..4a930ef 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -411,6 +411,9 @@ cfg80211_can_change_interface(struct cfg80211_registered_device *rdev,
 			      struct wireless_dev *wdev,
 			      enum nl80211_iftype iftype)
 {
+	/* TODO: For this function, we'll probably need to keep some
+	 * kind of interface combination check in cfg80211...
+	 */
 	return cfg80211_can_use_iftype_chan(rdev, wdev, iftype, NULL,
 					    CHAN_MODE_UNDEFINED, 0);
 }
@@ -425,16 +428,6 @@ cfg80211_can_add_interface(struct cfg80211_registered_device *rdev,
 	return cfg80211_can_change_interface(rdev, NULL, iftype);
 }
 
-static inline int
-cfg80211_can_use_chan(struct cfg80211_registered_device *rdev,
-		      struct wireless_dev *wdev,
-		      struct ieee80211_channel *chan,
-		      enum cfg80211_chan_mode chanmode)
-{
-	return cfg80211_can_use_iftype_chan(rdev, wdev, wdev->iftype,
-					    chan, chanmode, 0);
-}
-
 static inline unsigned int elapsed_jiffies_msecs(unsigned long start)
 {
 	unsigned long end = jiffies;
diff --git a/net/wireless/ibss.c b/net/wireless/ibss.c
index 349db9d..d81cb68 100644
--- a/net/wireless/ibss.c
+++ b/net/wireless/ibss.c
@@ -135,6 +135,10 @@ int __cfg80211_join_ibss(struct cfg80211_registered_device *rdev,
 		radar_detect_width = BIT(params->chandef.width);
 	}
 
+	/* TODO: We need to check the combinations at this point, we
+	 * probably must move this call down to join_ibss() in
+	 * mac80211.
+	 */
 	err = cfg80211_can_use_iftype_chan(rdev, wdev, wdev->iftype,
 					   check_chan,
 					   (params->channel_fixed &&
diff --git a/net/wireless/mesh.c b/net/wireless/mesh.c
index a95212b..b8a7a35 100644
--- a/net/wireless/mesh.c
+++ b/net/wireless/mesh.c
@@ -99,7 +99,6 @@ int __cfg80211_join_mesh(struct cfg80211_registered_device *rdev,
 			 const struct mesh_config *conf)
 {
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
-	u8 radar_detect_width = 0;
 	int err;
 
 	BUILD_BUG_ON(IEEE80211_MAX_SSID_LEN != IEEE80211_MAX_MESH_ID_LEN);
@@ -178,22 +177,6 @@ int __cfg80211_join_mesh(struct cfg80211_registered_device *rdev,
 	if (!cfg80211_reg_can_beacon(&rdev->wiphy, &setup->chandef))
 		return -EINVAL;
 
-	err = cfg80211_chandef_dfs_required(wdev->wiphy,
-					    &setup->chandef,
-					    NL80211_IFTYPE_MESH_POINT);
-	if (err < 0)
-		return err;
-
-	if (err > 0)
-		radar_detect_width = BIT(setup->chandef.width);
-
-	err = cfg80211_can_use_iftype_chan(rdev, wdev, wdev->iftype,
-					   setup->chandef.chan,
-					   CHAN_MODE_SHARED,
-					   radar_detect_width);
-	if (err)
-		return err;
-
 	err = rdev_join_mesh(rdev, dev, conf, setup);
 	if (!err) {
 		memcpy(wdev->ssid, setup->mesh_id, setup->mesh_id_len);
@@ -239,17 +222,6 @@ int cfg80211_set_mesh_channel(struct cfg80211_registered_device *rdev,
 		if (!netif_running(wdev->netdev))
 			return -ENETDOWN;
 
-		/* cfg80211_can_use_chan() calls
-		 * cfg80211_can_use_iftype_chan() with no radar
-		 * detection, so if we're trying to use a radar
-		 * channel here, something is wrong.
-		 */
-		WARN_ON_ONCE(chandef->chan->flags & IEEE80211_CHAN_RADAR);
-		err = cfg80211_can_use_chan(rdev, wdev, chandef->chan,
-					    CHAN_MODE_SHARED);
-		if (err)
-			return err;
-
 		err = rdev_libertas_set_mesh_channel(rdev, wdev->netdev,
 						     chandef->chan);
 		if (!err)
diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
index c52ff59..4b4ba70 100644
--- a/net/wireless/mlme.c
+++ b/net/wireless/mlme.c
@@ -233,14 +233,8 @@ int cfg80211_mlme_auth(struct cfg80211_registered_device *rdev,
 	if (!req.bss)
 		return -ENOENT;
 
-	err = cfg80211_can_use_chan(rdev, wdev, req.bss->channel,
-				    CHAN_MODE_SHARED);
-	if (err)
-		goto out;
-
 	err = rdev_auth(rdev, dev, &req);
 
-out:
 	cfg80211_put_bss(&rdev->wiphy, req.bss);
 	return err;
 }
@@ -306,16 +300,10 @@ int cfg80211_mlme_assoc(struct cfg80211_registered_device *rdev,
 	if (!req->bss)
 		return -ENOENT;
 
-	err = cfg80211_can_use_chan(rdev, wdev, chan, CHAN_MODE_SHARED);
-	if (err)
-		goto out;
-
 	err = rdev_assoc(rdev, dev, req);
 	if (!err)
 		cfg80211_hold_bss(bss_from_pub(req->bss));
-
-out:
-	if (err)
+	else
 		cfg80211_put_bss(&rdev->wiphy, req->bss);
 
 	return err;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 9fe7746..07253bf 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -3142,7 +3142,6 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	struct cfg80211_ap_settings params;
 	int err;
-	u8 radar_detect_width = 0;
 
 	if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP &&
 	    dev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO)
@@ -3261,24 +3260,6 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
 	if (!cfg80211_reg_can_beacon(&rdev->wiphy, &params.chandef))
 		return -EINVAL;
 
-	err = cfg80211_chandef_dfs_required(wdev->wiphy,
-					    &params.chandef,
-					    NL80211_IFTYPE_AP);
-	if (err < 0)
-		return err;
-
-	if (err > 0) {
-		params.radar_required = true;
-		radar_detect_width = BIT(params.chandef.width);
-	}
-
-	err = cfg80211_can_use_iftype_chan(rdev, wdev, wdev->iftype,
-					   params.chandef.chan,
-					   CHAN_MODE_SHARED,
-					   radar_detect_width);
-	if (err)
-		return err;
-
 	if (info->attrs[NL80211_ATTR_ACL_POLICY]) {
 		params.acl = parse_acl_data(&rdev->wiphy, info);
 		if (IS_ERR(params.acl))
@@ -5813,12 +5794,6 @@ static int nl80211_start_radar_detection(struct sk_buff *skb,
 	if (!rdev->ops->start_radar_detection)
 		return -EOPNOTSUPP;
 
-	err = cfg80211_can_use_iftype_chan(rdev, wdev, wdev->iftype,
-					   chandef.chan, CHAN_MODE_SHARED,
-					   BIT(chandef.width));
-	if (err)
-		return err;
-
 	cac_time_ms = cfg80211_chandef_dfs_cac_time(&rdev->wiphy, &chandef);
 	if (WARN_ON(!cac_time_ms))
 		cac_time_ms = IEEE80211_DFS_MIN_CAC_TIME_MS;
@@ -5946,6 +5921,10 @@ skip_beacons:
 		params.radar_required = true;
 	}
 
+	/* TODO: I left this here for now.  With channel switch, the
+	 * verification is a bit more complicated, because we only do
+	 * it later when the channel switch really happens.
+	 */
 	err = cfg80211_can_use_iftype_chan(rdev, wdev, wdev->iftype,
 					   params.chandef.chan,
 					   CHAN_MODE_SHARED,
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 7ecc0ef..b2498e3 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -1359,6 +1359,11 @@ int cfg80211_can_use_iftype_chan(struct cfg80211_registered_device *rdev,
 
 	num[iftype] = 1;
 
+	/* TODO: We'll probably not need this anymore, since this
+	 * should only be called with CHAN_MODE_UNDEFINED. There are
+	 * still a couple of pending calls where other chanmodes are
+	 * used, but we should get rid of them.
+	 */
 	switch (chanmode) {
 	case CHAN_MODE_UNDEFINED:
 		break;
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH v9 4/5] cfg80211/mac80211: move combination check to mac80211 for ibss
  2014-03-10 21:31 [PATCH v9 0/5] cfg80211/mac80211: move interface combinations check to mac80211 Luciano Coelho
                   ` (2 preceding siblings ...)
  2014-03-10 21:31 ` [PATCH v9 3/5] cfg80211/mac80211: move interface counting for combination check to mac80211 Luciano Coelho
@ 2014-03-10 21:31 ` Luciano Coelho
  2014-03-10 21:31 ` [PATCH v9 5/5] cfg80211/mac80211: move more combination checks to mac80211 Luciano Coelho
  4 siblings, 0 replies; 10+ messages in thread
From: Luciano Coelho @ 2014-03-10 21:31 UTC (permalink / raw)
  To: linux-wireless; +Cc: johannes, michal.kazior, sw, andrei.otcheretianski

Now that mac80211 can check the interface combinations itself, move
the combinations check from cfg80211 to mac80211 when joining an IBSS.

Signed-off-by: Luciano Coelho <luciano.coelho@intel.com>
---
In v2:

   * lock the chanctx mutex in ieee80211_ibss_join() before calling
     ieee80211_check_combinations(). (Thanks Michal);

   * pass the mode argument instead of IEEE80211_CHANCTX_SHARED to
     ieee80211_check_combinations() in ieee80211_vif_use_channel();

In v3:

   * moved the second change from v2 (pass the mode argument...) to
     the previous patch, where it should be;

In v4:

   * rebased on top of slightly modified applied patches

In v5:

   * use local and sdata in ibss when calling
     ieee80211_check_combinations()

In v7:

   * don't pass local in ieee80211_check_combinations() anymore;

In v8:

   * fix radar_detect_width before calling
     ieee80211_check_combinations() -- rebase bug;

In v9:

   * pass sdata->wdev.iftype instead of sdata->vif.type when calling
     cfg80211_chandef_dfs_required();
---
 net/mac80211/ibss.c | 32 +++++++++++++++++++++++++++++---
 net/wireless/ibss.c | 28 ----------------------------
 2 files changed, 29 insertions(+), 31 deletions(-)

diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 12057fd..72b9337 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -1644,7 +1644,33 @@ int ieee80211_ibss_join(struct ieee80211_sub_if_data *sdata,
 	u32 changed = 0;
 	u32 rate_flags;
 	struct ieee80211_supported_band *sband;
+	enum ieee80211_chanctx_mode chanmode;
+	struct ieee80211_local *local = sdata->local;
+	int radar_detect_width = 0;
 	int i;
+	int ret;
+
+	ret = cfg80211_chandef_dfs_required(local->hw.wiphy,
+					    &params->chandef,
+					    sdata->wdev.iftype);
+	if (ret < 0)
+		return ret;
+
+	if (ret > 0) {
+		if (!params->userspace_handles_dfs)
+			return -EINVAL;
+		radar_detect_width = BIT(params->chandef.width);
+	}
+
+	chanmode = (params->channel_fixed && !ret) ?
+		IEEE80211_CHANCTX_SHARED : IEEE80211_CHANCTX_EXCLUSIVE;
+
+	mutex_lock(&local->chanctx_mtx);
+	ret = ieee80211_check_combinations(sdata, &params->chandef, chanmode,
+					   radar_detect_width);
+	mutex_unlock(&local->chanctx_mtx);
+	if (ret < 0)
+		return ret;
 
 	if (params->bssid) {
 		memcpy(sdata->u.ibss.bssid, params->bssid, ETH_ALEN);
@@ -1659,7 +1685,7 @@ int ieee80211_ibss_join(struct ieee80211_sub_if_data *sdata,
 
 	/* fix basic_rates if channel does not support these rates */
 	rate_flags = ieee80211_chandef_rate_flags(&params->chandef);
-	sband = sdata->local->hw.wiphy->bands[params->chandef.chan->band];
+	sband = local->hw.wiphy->bands[params->chandef.chan->band];
 	for (i = 0; i < sband->n_bitrates; i++) {
 		if ((rate_flags & sband->bitrates[i].flags) != rate_flags)
 			sdata->u.ibss.basic_rates &= ~BIT(i);
@@ -1708,9 +1734,9 @@ int ieee80211_ibss_join(struct ieee80211_sub_if_data *sdata,
 	ieee80211_bss_info_change_notify(sdata, changed);
 
 	sdata->smps_mode = IEEE80211_SMPS_OFF;
-	sdata->needed_rx_chains = sdata->local->rx_chains;
+	sdata->needed_rx_chains = local->rx_chains;
 
-	ieee80211_queue_work(&sdata->local->hw, &sdata->work);
+	ieee80211_queue_work(&local->hw, &sdata->work);
 
 	return 0;
 }
diff --git a/net/wireless/ibss.c b/net/wireless/ibss.c
index d81cb68..8282de8 100644
--- a/net/wireless/ibss.c
+++ b/net/wireless/ibss.c
@@ -88,8 +88,6 @@ int __cfg80211_join_ibss(struct cfg80211_registered_device *rdev,
 			 struct cfg80211_cached_keys *connkeys)
 {
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
-	struct ieee80211_channel *check_chan;
-	u8 radar_detect_width = 0;
 	int err;
 
 	ASSERT_WDEV_LOCK(wdev);
@@ -126,32 +124,6 @@ int __cfg80211_join_ibss(struct cfg80211_registered_device *rdev,
 #ifdef CONFIG_CFG80211_WEXT
 	wdev->wext.ibss.chandef = params->chandef;
 #endif
-	check_chan = params->chandef.chan;
-	if (params->userspace_handles_dfs) {
-		/* Check for radar even if the current channel is not
-		 * a radar channel - it might decide to change to DFS
-		 * channel later.
-		 */
-		radar_detect_width = BIT(params->chandef.width);
-	}
-
-	/* TODO: We need to check the combinations at this point, we
-	 * probably must move this call down to join_ibss() in
-	 * mac80211.
-	 */
-	err = cfg80211_can_use_iftype_chan(rdev, wdev, wdev->iftype,
-					   check_chan,
-					   (params->channel_fixed &&
-					    !radar_detect_width)
-					   ? CHAN_MODE_SHARED
-					   : CHAN_MODE_EXCLUSIVE,
-					   radar_detect_width);
-
-	if (err) {
-		wdev->connect_keys = NULL;
-		return err;
-	}
-
 	err = rdev_join_ibss(rdev, dev, params);
 	if (err) {
 		wdev->connect_keys = NULL;
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH v9 5/5] cfg80211/mac80211: move more combination checks to mac80211
  2014-03-10 21:31 [PATCH v9 0/5] cfg80211/mac80211: move interface combinations check to mac80211 Luciano Coelho
                   ` (3 preceding siblings ...)
  2014-03-10 21:31 ` [PATCH v9 4/5] cfg80211/mac80211: move combination check to mac80211 for ibss Luciano Coelho
@ 2014-03-10 21:31 ` Luciano Coelho
  4 siblings, 0 replies; 10+ messages in thread
From: Luciano Coelho @ 2014-03-10 21:31 UTC (permalink / raw)
  To: linux-wireless; +Cc: johannes, michal.kazior, sw, andrei.otcheretianski

Get rid of the cfg80211_can_add_interface() and
cfg80211_can_change_interface() functions by moving that functionality
to mac80211.  With this patch all interface combination checks are now
out of cfg80211 (except for the channel switch case which will be
addressed in a future commit).

Additionally, modify the ieee80211_check_combinations() function so
that an undefined chandef can be passed, in order to use it before a
channel is defined.

Signed-off-by: Luciano Coelho <luciano.coelho@intel.com>
---
 net/mac80211/cfg.c     |  9 +++++++++
 net/mac80211/iface.c   |  6 +++++-
 net/mac80211/util.c    | 10 +++++++---
 net/wireless/core.c    | 11 +++--------
 net/wireless/core.h    | 22 ----------------------
 net/wireless/nl80211.c |  5 ++---
 net/wireless/util.c    |  5 -----
 7 files changed, 26 insertions(+), 42 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 9b555e0..fae76f7 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -109,6 +109,15 @@ static int ieee80211_change_iface(struct wiphy *wiphy,
 static int ieee80211_start_p2p_device(struct wiphy *wiphy,
 				      struct wireless_dev *wdev)
 {
+	struct ieee80211_sub_if_data *sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);
+	int ret;
+
+	mutex_lock(&sdata->local->chanctx_mtx);
+	ret = ieee80211_check_combinations(sdata, NULL, 0, 0);
+	mutex_unlock(&sdata->local->chanctx_mtx);
+	if (ret < 0)
+		return ret;
+
 	return ieee80211_do_open(wdev, true);
 }
 
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index be198f4..4c55481 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -250,6 +250,7 @@ static int ieee80211_check_concurrent_iface(struct ieee80211_sub_if_data *sdata,
 {
 	struct ieee80211_local *local = sdata->local;
 	struct ieee80211_sub_if_data *nsdata;
+	int ret;
 
 	ASSERT_RTNL();
 
@@ -300,7 +301,10 @@ static int ieee80211_check_concurrent_iface(struct ieee80211_sub_if_data *sdata,
 		}
 	}
 
-	return 0;
+	mutex_lock(&local->chanctx_mtx);
+	ret = ieee80211_check_combinations(sdata, NULL, 0, 0);
+	mutex_unlock(&local->chanctx_mtx);
+	return ret;
 }
 
 static int ieee80211_check_queues(struct ieee80211_sub_if_data *sdata,
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 3972b64..955b043 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -2812,7 +2812,7 @@ int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
 	enum nl80211_iftype iftype = sdata->wdev.iftype;
 	int num[NUM_NL80211_IFTYPES];
 	struct ieee80211_chanctx *ctx;
-	int num_different_channels = 1;
+	int num_different_channels = 0;
 	int total = 1;
 
 	lockdep_assert_held(&local->chanctx_mtx);
@@ -2820,9 +2820,13 @@ int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
 	if (WARN_ON(hweight32(radar_detect) > 1))
 		return -EINVAL;
 
-	if (WARN_ON(chanmode == IEEE80211_CHANCTX_SHARED && !chandef->chan))
+	if (WARN_ON(chandef && chanmode == IEEE80211_CHANCTX_SHARED &&
+		    !chandef->chan))
 		return -EINVAL;
 
+	if (chandef)
+		num_different_channels = 1;
+
 	if (WARN_ON(iftype >= NUM_NL80211_IFTYPES))
 		return -EINVAL;
 
@@ -2845,7 +2849,7 @@ int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
 			num_different_channels++;
 			continue;
 		}
-		if ((chanmode == IEEE80211_CHANCTX_SHARED) &&
+		if (chandef && chanmode == IEEE80211_CHANCTX_SHARED &&
 		    cfg80211_chandef_compatible(chandef,
 						&ctx->conf.def))
 			continue;
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 276cf93..7fca03e 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -396,10 +396,7 @@ static int wiphy_verify_combinations(struct wiphy *wiphy)
 		for (j = 0; j < c->n_limits; j++) {
 			u16 types = c->limits[j].types;
 
-			/*
-			 * interface types shouldn't overlap, this is
-			 * used in cfg80211_can_change_interface()
-			 */
+			/* interface types shouldn't overlap */
 			if (WARN_ON(types & all_iftypes))
 				return -EINVAL;
 			all_iftypes |= types;
@@ -798,7 +795,6 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	struct cfg80211_registered_device *rdev;
-	int ret;
 
 	if (!wdev)
 		return NOTIFY_DONE;
@@ -961,9 +957,8 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 	case NETDEV_PRE_UP:
 		if (!(wdev->wiphy->interface_modes & BIT(wdev->iftype)))
 			return notifier_from_errno(-EOPNOTSUPP);
-		ret = cfg80211_can_add_interface(rdev, wdev->iftype);
-		if (ret)
-			return notifier_from_errno(ret);
+		if (rfkill_blocked(rdev->rfkill))
+			return notifier_from_errno(-ERFKILL);
 		break;
 	}
 
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 4a930ef..53f1996 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -406,28 +406,6 @@ unsigned int
 cfg80211_chandef_dfs_cac_time(struct wiphy *wiphy,
 			      const struct cfg80211_chan_def *chandef);
 
-static inline int
-cfg80211_can_change_interface(struct cfg80211_registered_device *rdev,
-			      struct wireless_dev *wdev,
-			      enum nl80211_iftype iftype)
-{
-	/* TODO: For this function, we'll probably need to keep some
-	 * kind of interface combination check in cfg80211...
-	 */
-	return cfg80211_can_use_iftype_chan(rdev, wdev, iftype, NULL,
-					    CHAN_MODE_UNDEFINED, 0);
-}
-
-static inline int
-cfg80211_can_add_interface(struct cfg80211_registered_device *rdev,
-			   enum nl80211_iftype iftype)
-{
-	if (rfkill_blocked(rdev->rfkill))
-		return -ERFKILL;
-
-	return cfg80211_can_change_interface(rdev, NULL, iftype);
-}
-
 static inline unsigned int elapsed_jiffies_msecs(unsigned long start)
 {
 	unsigned long end = jiffies;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 07253bf..5465e0e 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -8957,9 +8957,8 @@ static int nl80211_start_p2p_device(struct sk_buff *skb, struct genl_info *info)
 	if (wdev->p2p_started)
 		return 0;
 
-	err = cfg80211_can_add_interface(rdev, wdev->iftype);
-	if (err)
-		return err;
+	if (rfkill_blocked(rdev->rfkill))
+		return -ERFKILL;
 
 	err = rdev_start_p2p_device(rdev, wdev);
 	if (err)
diff --git a/net/wireless/util.c b/net/wireless/util.c
index b2498e3..378038d30 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -873,11 +873,6 @@ int cfg80211_change_iface(struct cfg80211_registered_device *rdev,
 		return -EBUSY;
 
 	if (ntype != otype && netif_running(dev)) {
-		err = cfg80211_can_change_interface(rdev, dev->ieee80211_ptr,
-						    ntype);
-		if (err)
-			return err;
-
 		dev->ieee80211_ptr->use_4addr = false;
 		dev->ieee80211_ptr->mesh_id_up_len = 0;
 		wdev_lock(dev->ieee80211_ptr);
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [PATCH v9 1/5] cfg80211: refactor cfg80211_can_use_iftype_chan()
  2014-03-10 21:31 ` [PATCH v9 1/5] cfg80211: refactor cfg80211_can_use_iftype_chan() Luciano Coelho
@ 2014-03-11 10:40   ` Eliad Peller
  2014-03-11 13:09     ` Luca Coelho
  0 siblings, 1 reply; 10+ messages in thread
From: Eliad Peller @ 2014-03-11 10:40 UTC (permalink / raw)
  To: Luciano Coelho
  Cc: linux-wireless, Johannes Berg, Michal Kazior, sw, andrei.otcheretianski

On Mon, Mar 10, 2014 at 11:31 PM, Luciano Coelho
<luciano.coelho@intel.com> wrote:
> Separate the code that counts the interface types and channels from
> the code that check the interface combinations.  The new function that
> checks for combinations is exported so it can be called by the
> drivers.
>
> This is done in preparation for moving the interface combinations
> checks out of cfg80211.
>
> Signed-off-by: Luciano Coelho <luciano.coelho@intel.com>
> ---
[...]

> +int cfg80211_check_combinations(struct wiphy *wiphy,
> +                               const int num_different_channels,
> +                               const u8 radar_detect,
> +                               const int iftype_num[NUM_NL80211_IFTYPES])
> +{
> +       int i, j, iftype;
> +       int num_interfaces = 0;
> +       u32 used_iftypes = 0;
> +
> +       for (iftype = 0; iftype < NUM_NL80211_IFTYPES; iftype++) {
> +               num_interfaces += iftype_num[iftype];
> +               if (iftype_num[iftype] > 0)
> +                       used_iftypes |= BIT(iftype);
> +       }
> +
[...]

> +
> +               for (iftype = 0; iftype < NUM_NL80211_IFTYPES; iftype++) {
> +                       if (wiphy->software_iftypes & BIT(iftype))
> +                               continue;
> +                       for (j = 0; j < c->n_limits; j++) {
> +                               all_iftypes |= limits[j].types;
> +                               if (!(limits[j].types & BIT(iftype)))
> +                                       continue;
> +                               if (limits[j].max < iftype_num[iftype])
> +                                       goto cont;
> +                               limits[j].max -= iftype_num[iftype];
> +                       }
> +               }
> +
> +               if (radar_detect && !(c->radar_detect_widths & radar_detect))
> +                       goto cont;
> +
> +               /* Finally check that all iftypes that we're currently
> +                * using are actually part of this combination. If they
> +                * aren't then we can't use this combination and have
> +                * to continue to the next.
> +                */
> +               if ((all_iftypes & used_iftypes) != used_iftypes)
> +                       goto cont;
> +
if software_iftypes will be passed to this function, this check will
fail (as they exist only in used_iftypes).

Eliad.

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v9 2/5] cfg80211/mac80211: refactor cfg80211_chandef_dfs_required()
  2014-03-10 21:31 ` [PATCH v9 2/5] cfg80211/mac80211: refactor cfg80211_chandef_dfs_required() Luciano Coelho
@ 2014-03-11 10:45   ` Eliad Peller
  2014-03-11 13:40     ` Luca Coelho
  0 siblings, 1 reply; 10+ messages in thread
From: Eliad Peller @ 2014-03-11 10:45 UTC (permalink / raw)
  To: Luciano Coelho
  Cc: linux-wireless, Johannes Berg, Michal Kazior, sw, andrei.otcheretianski

On Mon, Mar 10, 2014 at 11:31 PM, Luciano Coelho
<luciano.coelho@intel.com> wrote:
> Some interface types don't require DFS (such as STATION, P2P_CLIENT
> etc).  In order to centralize these decisions, make
> cfg80211_chandef_dfs_required() take the iftype into consideration.
>
> Signed-off-by: Luciano Coelho <luciano.coelho@intel.com>
> ---
[...]

>  int cfg80211_chandef_dfs_required(struct wiphy *wiphy,
> -                                 const struct cfg80211_chan_def *chandef)
> +                                 const struct cfg80211_chan_def *chandef,
> +                                 enum nl80211_iftype iftype)
>  {
>         int width;
> -       int r;
> +       int ret;
>
>         if (WARN_ON(!cfg80211_chandef_valid(chandef)))
>                 return -EINVAL;
>
> -       width = cfg80211_chandef_get_width(chandef);
> -       if (width < 0)
> -               return -EINVAL;
> +       switch (iftype) {
> +       case NL80211_IFTYPE_ADHOC:
> +       case NL80211_IFTYPE_AP:
> +       case NL80211_IFTYPE_P2P_GO:
> +       case NL80211_IFTYPE_MESH_POINT:
> +               width = cfg80211_chandef_get_width(chandef);
> +               if (width < 0)
> +                       return -EINVAL;
>
it's a matter of style, but i think bailing out in non-relevant cases
and taking the code out of the switch looks better (less indentation,
etc.).

[...]

> +       case NL80211_IFTYPE_STATION:
> +       case NL80211_IFTYPE_P2P_CLIENT:
> +       case NL80211_IFTYPE_MONITOR:
> +       case NL80211_IFTYPE_AP_VLAN:
> +       case NL80211_IFTYPE_WDS:
> +       case NL80211_IFTYPE_P2P_DEVICE:
> +               break;
> +       case NL80211_IFTYPE_UNSPECIFIED:
> +       case NUM_NL80211_IFTYPES:
> +               WARN_ON(1);
> +       }

[...]

> @@ -671,7 +700,8 @@ bool cfg80211_reg_can_beacon(struct wiphy *wiphy,
> -       if (cfg80211_chandef_dfs_required(wiphy, chandef) > 0 &&
> +       if (cfg80211_chandef_dfs_required(wiphy, chandef,
> +                                         NL80211_IFTYPE_UNSPECIFIED) > 0 &&
wouldn't this WARN_ON()?

> @@ -5796,7 +5799,8 @@ static int nl80211_start_radar_detection(struct sk_buff *skb,
>         if (wdev->cac_started)
>                 return -EBUSY;
>
> -       err = cfg80211_chandef_dfs_required(wdev->wiphy, &chandef);
> +       err = cfg80211_chandef_dfs_required(wdev->wiphy, &chandef,
> +                                           NL80211_IFTYPE_UNSPECIFIED);
ditto.

Eliad.

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v9 1/5] cfg80211: refactor cfg80211_can_use_iftype_chan()
  2014-03-11 10:40   ` Eliad Peller
@ 2014-03-11 13:09     ` Luca Coelho
  0 siblings, 0 replies; 10+ messages in thread
From: Luca Coelho @ 2014-03-11 13:09 UTC (permalink / raw)
  To: Eliad Peller
  Cc: linux-wireless, Johannes Berg, Michal Kazior, sw, andrei.otcheretianski

On Tue, 2014-03-11 at 12:40 +0200, Eliad Peller wrote:
> On Mon, Mar 10, 2014 at 11:31 PM, Luciano Coelho
> <luciano.coelho@intel.com> wrote:
> > Separate the code that counts the interface types and channels from
> > the code that check the interface combinations.  The new function that
> > checks for combinations is exported so it can be called by the
> > drivers.
> >
> > This is done in preparation for moving the interface combinations
> > checks out of cfg80211.
> >
> > Signed-off-by: Luciano Coelho <luciano.coelho@intel.com>
> > ---
> [...]
> 
> > +int cfg80211_check_combinations(struct wiphy *wiphy,
> > +                               const int num_different_channels,
> > +                               const u8 radar_detect,
> > +                               const int iftype_num[NUM_NL80211_IFTYPES])
> > +{
> > +       int i, j, iftype;
> > +       int num_interfaces = 0;
> > +       u32 used_iftypes = 0;
> > +
> > +       for (iftype = 0; iftype < NUM_NL80211_IFTYPES; iftype++) {
> > +               num_interfaces += iftype_num[iftype];
> > +               if (iftype_num[iftype] > 0)
> > +                       used_iftypes |= BIT(iftype);
> > +       }
> > +
> [...]
> 
> > +
> > +               for (iftype = 0; iftype < NUM_NL80211_IFTYPES; iftype++) {
> > +                       if (wiphy->software_iftypes & BIT(iftype))
> > +                               continue;
> > +                       for (j = 0; j < c->n_limits; j++) {
> > +                               all_iftypes |= limits[j].types;
> > +                               if (!(limits[j].types & BIT(iftype)))
> > +                                       continue;
> > +                               if (limits[j].max < iftype_num[iftype])
> > +                                       goto cont;
> > +                               limits[j].max -= iftype_num[iftype];
> > +                       }
> > +               }
> > +
> > +               if (radar_detect && !(c->radar_detect_widths & radar_detect))
> > +                       goto cont;
> > +
> > +               /* Finally check that all iftypes that we're currently
> > +                * using are actually part of this combination. If they
> > +                * aren't then we can't use this combination and have
> > +                * to continue to the next.
> > +                */
> > +               if ((all_iftypes & used_iftypes) != used_iftypes)
> > +                       goto cont;
> > +
> if software_iftypes will be passed to this function, this check will
> fail (as they exist only in used_iftypes).

Good point, I guess I should skip SW interface types in the iterator
where I mark the user interfaces above...

--
Luca.


^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v9 2/5] cfg80211/mac80211: refactor cfg80211_chandef_dfs_required()
  2014-03-11 10:45   ` Eliad Peller
@ 2014-03-11 13:40     ` Luca Coelho
  0 siblings, 0 replies; 10+ messages in thread
From: Luca Coelho @ 2014-03-11 13:40 UTC (permalink / raw)
  To: Eliad Peller
  Cc: linux-wireless, Johannes Berg, Michal Kazior, sw, andrei.otcheretianski

On Tue, 2014-03-11 at 12:45 +0200, Eliad Peller wrote:
> On Mon, Mar 10, 2014 at 11:31 PM, Luciano Coelho
> <luciano.coelho@intel.com> wrote:
> > Some interface types don't require DFS (such as STATION, P2P_CLIENT
> > etc).  In order to centralize these decisions, make
> > cfg80211_chandef_dfs_required() take the iftype into consideration.
> >
> > Signed-off-by: Luciano Coelho <luciano.coelho@intel.com>
> > ---
> [...]
> 
> >  int cfg80211_chandef_dfs_required(struct wiphy *wiphy,
> > -                                 const struct cfg80211_chan_def *chandef)
> > +                                 const struct cfg80211_chan_def *chandef,
> > +                                 enum nl80211_iftype iftype)
> >  {
> >         int width;
> > -       int r;
> > +       int ret;
> >
> >         if (WARN_ON(!cfg80211_chandef_valid(chandef)))
> >                 return -EINVAL;
> >
> > -       width = cfg80211_chandef_get_width(chandef);
> > -       if (width < 0)
> > -               return -EINVAL;
> > +       switch (iftype) {
> > +       case NL80211_IFTYPE_ADHOC:
> > +       case NL80211_IFTYPE_AP:
> > +       case NL80211_IFTYPE_P2P_GO:
> > +       case NL80211_IFTYPE_MESH_POINT:
> > +               width = cfg80211_chandef_get_width(chandef);
> > +               if (width < 0)
> > +                       return -EINVAL;
> >
> it's a matter of style, but i think bailing out in non-relevant cases
> and taking the code out of the switch looks better (less indentation,
> etc.).

Yeah, it's a style issue and I tend to agree with you.  But I also think
it's good to have a switch where we can easily see what happens with
each interface type.

I won't change this now, but we can refactor this at a later point if it
starts getting annoying. ;)



> [...]
> 
> > +       case NL80211_IFTYPE_STATION:
> > +       case NL80211_IFTYPE_P2P_CLIENT:
> > +       case NL80211_IFTYPE_MONITOR:
> > +       case NL80211_IFTYPE_AP_VLAN:
> > +       case NL80211_IFTYPE_WDS:
> > +       case NL80211_IFTYPE_P2P_DEVICE:
> > +               break;
> > +       case NL80211_IFTYPE_UNSPECIFIED:
> > +       case NUM_NL80211_IFTYPES:
> > +               WARN_ON(1);
> > +       }
> 
> [...]
> 
> > @@ -671,7 +700,8 @@ bool cfg80211_reg_can_beacon(struct wiphy *wiphy,
> > -       if (cfg80211_chandef_dfs_required(wiphy, chandef) > 0 &&
> > +       if (cfg80211_chandef_dfs_required(wiphy, chandef,
> > +                                         NL80211_IFTYPE_UNSPECIFIED) > 0 &&
> wouldn't this WARN_ON()?

Yep, you're right.  Previously I was calling this and I didn't really
care what was the interface type.  But Johannes asked me to change the
cfg80211_dfs_required() code to warn if unspecified was passed and I
forgot to change it here.  I'll either have to add the iftype to the
cfg80211_reg_can_beacon() function arguments (which is a bit ugly,
because it's not really needed) or I'll have to move the UNSPECIFIED to
some sort of "don't care" in the cfg80211_chandef_dfs_required()
function.


> > @@ -5796,7 +5799,8 @@ static int nl80211_start_radar_detection(struct sk_buff *skb,
> >         if (wdev->cac_started)
> >                 return -EBUSY;
> >
> > -       err = cfg80211_chandef_dfs_required(wdev->wiphy, &chandef);
> > +       err = cfg80211_chandef_dfs_required(wdev->wiphy, &chandef,
> > +                                           NL80211_IFTYPE_UNSPECIFIED);
> ditto.

Ditto. :)

--
Cheers,
Luca.


^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-03-11 13:41 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-10 21:31 [PATCH v9 0/5] cfg80211/mac80211: move interface combinations check to mac80211 Luciano Coelho
2014-03-10 21:31 ` [PATCH v9 1/5] cfg80211: refactor cfg80211_can_use_iftype_chan() Luciano Coelho
2014-03-11 10:40   ` Eliad Peller
2014-03-11 13:09     ` Luca Coelho
2014-03-10 21:31 ` [PATCH v9 2/5] cfg80211/mac80211: refactor cfg80211_chandef_dfs_required() Luciano Coelho
2014-03-11 10:45   ` Eliad Peller
2014-03-11 13:40     ` Luca Coelho
2014-03-10 21:31 ` [PATCH v9 3/5] cfg80211/mac80211: move interface counting for combination check to mac80211 Luciano Coelho
2014-03-10 21:31 ` [PATCH v9 4/5] cfg80211/mac80211: move combination check to mac80211 for ibss Luciano Coelho
2014-03-10 21:31 ` [PATCH v9 5/5] cfg80211/mac80211: move more combination checks to mac80211 Luciano Coelho

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.