diff mbox

[6/7] mac80211: deny attempts at using chanctx during CSA

Message ID 1390227670-19030-7-git-send-email-michal.kazior@tieto.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Michal Kazior Jan. 20, 2014, 2:21 p.m. UTC
It was possible to connect STA when AP CSA was in
progress which clearly was a bug.

Deny attempts to increase chanctx refcount or
create chanctx.

This effectively denies starting a new interface
and radar detection while CSA is in progress.

Existing interfaces (including those with active
CSA) can be stopped though.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/mac80211/chan.c        |  7 +++++++
 net/mac80211/ieee80211_i.h |  1 +
 net/mac80211/util.c        | 18 ++++++++++++++++++
 3 files changed, 26 insertions(+)

Comments

Luca Coelho Jan. 20, 2014, 9:41 p.m. UTC | #1
On Mon, 2014-01-20 at 15:21 +0100, Michal Kazior wrote:
> It was possible to connect STA when AP CSA was in
> progress which clearly was a bug.

I understand that preventing this simplifies things, but I don't think
it's a bug.  If the CSA process takes a long time (ie. several TBTTs),
why not let a new STA associate meanwhile? The new STA knows when to
switch, since it can see the count in the beacons (and probe_responses).

--
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
Michal Kazior Jan. 21, 2014, 6:07 a.m. UTC | #2
On 20 January 2014 22:41, Luca Coelho <luca@coelho.fi> wrote:
> On Mon, 2014-01-20 at 15:21 +0100, Michal Kazior wrote:
>> It was possible to connect STA when AP CSA was in
>> progress which clearly was a bug.
>
> I understand that preventing this simplifies things, but I don't think
> it's a bug.  If the CSA process takes a long time (ie. several TBTTs),
> why not let a new STA associate meanwhile? The new STA knows when to
> switch, since it can see the count in the beacons (and probe_responses).

This patch prevents a station interface from connecting/associating
somewhere. It does not prevent a station from connecting to your AP
while it's in CSA. I probably should rephrase the commit message to
make it clear.


Micha?
--
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
Johannes Berg Jan. 21, 2014, 3:14 p.m. UTC | #3
On Mon, 2014-01-20 at 15:21 +0100, Michal Kazior wrote:
> It was possible to connect STA when AP CSA was in
> progress which clearly was a bug.
> 
> Deny attempts to increase chanctx refcount or
> create chanctx.
> 
> This effectively denies starting a new interface
> and radar detection while CSA is in progress.
> 
> Existing interfaces (including those with active
> CSA) can be stopped though.

I think this is the wrong approach. This makes more special cases for
non-chanctx code, whereas I think the code should gain more channel
context awareness. Luca has a patch to make channel switch work with
channel contexts (by creating one on the new channel and then later
switching to it), and I think this mechanism should be subsumed into the
channel context reservation code, rather than being CSA specific.

However, if there's only one channel context (or no support for channel
contexts) then CSA becomes a special case in Luca's patch, I guess,
because we can't be reserving a new one?

Luca, this is probably something you need to look into - this goes back
to the exact timing thing we just talked about. If we don't support
channel contexts, then we can create a fake one to switch to, but then
this fake one can't accept any further interfaces being bound to it
since it will not be able to coexist with the other one.

As far as this particular behaviour is concerned, I would then say that
then the old channel context should also be marked as incompatible for
the time of the CSA, so that no other (station) vif can attempt to use
it.

That'll result in a better solution overall, I think.

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
diff mbox

Patch

diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index f43613a..9057b66 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -517,6 +517,13 @@  int ieee80211_vif_use_channel(struct ieee80211_sub_if_data *sdata,
 
 	WARN_ON(sdata->dev && netif_carrier_ok(sdata->dev));
 
+	/* Do not allow an interface to bind to channel contexts during CSA as
+	 * it would end up with the interface being dragged to a different
+	 * channel once CSA completes. It's safe to unbind from channel
+	 * contexts though. */
+	if (ieee80211_is_csa_active(local))
+		return -EBUSY;
+
 	mutex_lock(&local->chanctx_mtx);
 	__ieee80211_vif_release_channel(sdata);
 
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index a6cda02..763a0ca 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1465,6 +1465,7 @@  int __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
 int ieee80211_channel_switch(struct wiphy *wiphy,
 			     struct cfg80211_csa_settings *params,
 			     int num_params);
+bool ieee80211_is_csa_active(struct ieee80211_local *local);
 
 /* interface handling */
 int ieee80211_iface_init(void);
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 676dc09..3e77c41 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -2775,3 +2775,21 @@  void ieee80211_recalc_dtim(struct ieee80211_local *local,
 
 	ps->dtim_count = dtim_count;
 }
+
+bool ieee80211_is_csa_active(struct ieee80211_local *local)
+{
+	struct ieee80211_sub_if_data *sdata;
+
+	lockdep_assert_held(&local->mtx);
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(sdata, &local->interfaces, list) {
+		if (sdata->vif.csa_active) {
+			rcu_read_unlock();
+			return true;
+		}
+	}
+	rcu_read_unlock();
+
+	return false;
+}