diff mbox

[v3,7/7] cfg80211/mac80211: move combination check to mac80211 for ibss

Message ID 1392906986-12275-8-git-send-email-luca@coelho.fi (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Luca Coelho Feb. 20, 2014, 2:36 p.m. UTC
From: Luciano Coelho <luciano.coelho@intel.com>

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;
---
 net/mac80211/ibss.c | 30 +++++++++++++++++++++++++++---
 net/wireless/ibss.c | 27 ---------------------------
 2 files changed, 27 insertions(+), 30 deletions(-)

Comments

Johannes Berg Feb. 21, 2014, 8:48 a.m. UTC | #1
On Thu, 2014-02-20 at 16:36 +0200, Luciano Coelho wrote:
> From: Luciano Coelho <luciano.coelho@intel.com>
> 
> Now that mac80211 can check the interface combinations itself, move
> the combinations check from cfg80211 to mac80211 when joining an IBSS.

> +++ b/net/mac80211/ibss.c
> @@ -1646,7 +1646,31 @@ 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;
>  	int i;
> +	int ret;
> +
> +	radar_detect_width = cfg80211_chandef_dfs_required(local->hw.wiphy,
> +							   &params->chandef,
> +							   sdata->vif.type);
> +	if (radar_detect_width < 0)
> +		return radar_detect_width;
> +
> +	if (radar_detect_width > 0 && !params->userspace_handles_dfs)
> +		return -EINVAL;
> +
> +	chanmode = (params->channel_fixed && !radar_detect_width) ?
> +		IEEE80211_CHANCTX_SHARED : IEEE80211_CHANCTX_EXCLUSIVE;
> +
> +	mutex_lock(&local->chanctx_mtx);
> +	ret = ieee80211_check_combinations(local->hw.wiphy, &sdata->wdev,
> +					   &params->chandef, chanmode,
> +					   radar_detect_width);
> +	mutex_unlock(&local->chanctx_mtx);
> +	if (ret < 0)
> +		return ret;

Hm, that's a lot of new code just for IBSS. Is there anything that could
be shared with other pieces of mac80211?

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Luca Coelho Feb. 24, 2014, 8:40 p.m. UTC | #2
On Fri, 2014-02-21 at 09:48 +0100, Johannes Berg wrote:
> On Thu, 2014-02-20 at 16:36 +0200, Luciano Coelho wrote:
> > From: Luciano Coelho <luciano.coelho@intel.com>
> > 
> > Now that mac80211 can check the interface combinations itself, move
> > the combinations check from cfg80211 to mac80211 when joining an IBSS.
> 
> > +++ b/net/mac80211/ibss.c
> > @@ -1646,7 +1646,31 @@ 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;
> >  	int i;
> > +	int ret;
> > +
> > +	radar_detect_width = cfg80211_chandef_dfs_required(local->hw.wiphy,
> > +							   &params->chandef,
> > +							   sdata->vif.type);
> > +	if (radar_detect_width < 0)
> > +		return radar_detect_width;
> > +
> > +	if (radar_detect_width > 0 && !params->userspace_handles_dfs)
> > +		return -EINVAL;
> > +
> > +	chanmode = (params->channel_fixed && !radar_detect_width) ?
> > +		IEEE80211_CHANCTX_SHARED : IEEE80211_CHANCTX_EXCLUSIVE;
> > +
> > +	mutex_lock(&local->chanctx_mtx);
> > +	ret = ieee80211_check_combinations(local->hw.wiphy, &sdata->wdev,
> > +					   &params->chandef, chanmode,
> > +					   radar_detect_width);
> > +	mutex_unlock(&local->chanctx_mtx);
> > +	if (ret < 0)
> > +		return ret;
> 
> Hm, that's a lot of new code just for IBSS. Is there anything that could
> be shared with other pieces of mac80211?

It's some extra code, right, but it's not that complicated and the two
things we do differently than in other parts is something we need to do
between the function calls (ie. we need radar_detect_width).   I think
it would get more complicated to try to change that.  Unless I add some
interface type knowledge into ieee80211_check_combinations(), which I'd
rather not do.

OTOH, as I discussed with Michal earlier, we don't strictly need these
calls here, since we will make the checks again when we actually join
the IBSS, where ieee80211_vif_use_channel() is called.  We need the
second check, because something may have changed in between.  But if
that second check fails, we don't return any error to the userspace, it
will just not work (and probably timeout).

I'll leave it as it is for v4, but I can change as you like and send a
new version later.

--
Luca.

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

Patch

diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 56b0dce..cf83763 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -1646,7 +1646,31 @@  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;
 	int i;
+	int ret;
+
+	radar_detect_width = cfg80211_chandef_dfs_required(local->hw.wiphy,
+							   &params->chandef,
+							   sdata->vif.type);
+	if (radar_detect_width < 0)
+		return radar_detect_width;
+
+	if (radar_detect_width > 0 && !params->userspace_handles_dfs)
+		return -EINVAL;
+
+	chanmode = (params->channel_fixed && !radar_detect_width) ?
+		IEEE80211_CHANCTX_SHARED : IEEE80211_CHANCTX_EXCLUSIVE;
+
+	mutex_lock(&local->chanctx_mtx);
+	ret = ieee80211_check_combinations(local->hw.wiphy, &sdata->wdev,
+					   &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);
@@ -1661,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);
@@ -1710,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 6e98e49..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,31 +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;