diff mbox

[1/3] mac80211: consider only relevant vifs for radar_required calculation

Message ID 1420366399-31535-1-git-send-email-eliad@wizery.com (mailing list archive)
State Accepted
Delegated to: Johannes Berg
Headers show

Commit Message

Eliad Peller Jan. 4, 2015, 10:13 a.m. UTC
no need to consider all the vifs, but only the ones
that have this context assigned.

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
This patch was already sent previously as part of a different patchset
(in which the other patch was later dropped).
However, i don't see it in patchwork, so resend it.

 net/mac80211/chan.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

Comments

Johannes Berg Jan. 6, 2015, 11:04 a.m. UTC | #1
all three applied.

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
Johannes Berg Jan. 6, 2015, 2:05 p.m. UTC | #2
On Tue, 2015-01-06 at 12:04 +0100, Johannes Berg wrote:
> all three applied.

Nope, dropped patch 2 again - it was causing issues when running the
hwsim tests:

$ ./vm-run.sh dfs ap_ht_40mhz_intolerant_ap
Starting test run in a virtual machine
./run-all.sh: passing the following args to run-tests.py: dfs
ap_ht_40mhz_intolerant_ap 
START dfs 1/2
19
703
PASS dfs 65.148949 2015-01-06 14:02:54.263314
START ap_ht_40mhz_intolerant_ap 2/2
19
719
command failed: Device or resource busy (-16)
Exception: AP startup failed
FAIL ap_ht_40mhz_intolerant_ap 15.53641 2015-01-06 14:03:09.842131
failed tests: ap_ht_40mhz_intolerant_ap

Maybe the tests don't clean up properly, or something, but I'm not going
to investigate this now.

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
Johannes Berg Jan. 6, 2015, 2:14 p.m. UTC | #3
On Tue, 2015-01-06 at 15:05 +0100, Johannes Berg wrote:
> On Tue, 2015-01-06 at 12:04 +0100, Johannes Berg wrote:
> > all three applied.
> 
> Nope, dropped patch 2 again

And dropped the other two also - with them the sequence of two tests I
was looking at never completes.

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
Eliad Peller Jan. 6, 2015, 6:57 p.m. UTC | #4
On Tue, Jan 6, 2015 at 4:14 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2015-01-06 at 15:05 +0100, Johannes Berg wrote:
>> On Tue, 2015-01-06 at 12:04 +0100, Johannes Berg wrote:
>> > all three applied.
>>
>> Nope, dropped patch 2 again
>
> And dropped the other two also - with them the sequence of two tests I
> was looking at never completes.
>
err... sorry about that.
i'll look into it.

thanks,
Eliad.
--
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. 6, 2015, 7:02 p.m. UTC | #5
On Tue, 2015-01-06 at 20:57 +0200, Eliad Peller wrote:
> On Tue, Jan 6, 2015 at 4:14 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> > On Tue, 2015-01-06 at 15:05 +0100, Johannes Berg wrote:
> >> On Tue, 2015-01-06 at 12:04 +0100, Johannes Berg wrote:
> >> > all three applied.
> >>
> >> Nope, dropped patch 2 again
> >
> > And dropped the other two also - with them the sequence of two tests I
> > was looking at never completes.
> >
> err... sorry about that.
> i'll look into it.

I think there's probably two bugs - the test suite should wait for all
the DFS stuff to finish.

But the fact that it never completes now is strange, maybe somehow radar
detection doesn't get turned off properly after your patches?

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
Eliad Peller Jan. 6, 2015, 7:36 p.m. UTC | #6
On Tue, Jan 6, 2015 at 9:02 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2015-01-06 at 20:57 +0200, Eliad Peller wrote:
>> On Tue, Jan 6, 2015 at 4:14 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
>> > On Tue, 2015-01-06 at 15:05 +0100, Johannes Berg wrote:
>> >> On Tue, 2015-01-06 at 12:04 +0100, Johannes Berg wrote:
>> >> > all three applied.
>> >>
>> >> Nope, dropped patch 2 again
>> >
>> > And dropped the other two also - with them the sequence of two tests I
>> > was looking at never completes.
>> >
>> err... sorry about that.
>> i'll look into it.
>
> I think there's probably two bugs - the test suite should wait for all
> the DFS stuff to finish.
>
> But the fact that it never completes now is strange, maybe somehow radar
> detection doesn't get turned off properly after your patches?
>
it's probably local->radar_detect_enabled doesn't get cleared.
patch (3) fixes a similar issue, but there seems to be another one.
(it seems that patch (1) exposed these issues, so i should probably
reorder the patches and make it the last one)

Eliad.
--
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
Eliad Peller Jan. 7, 2015, 3:58 p.m. UTC | #7
On Tue, Jan 6, 2015 at 9:36 PM, Eliad Peller <eliad@wizery.com> wrote:
> On Tue, Jan 6, 2015 at 9:02 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
>> On Tue, 2015-01-06 at 20:57 +0200, Eliad Peller wrote:
>>> On Tue, Jan 6, 2015 at 4:14 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
>>> > On Tue, 2015-01-06 at 15:05 +0100, Johannes Berg wrote:
>>> >> On Tue, 2015-01-06 at 12:04 +0100, Johannes Berg wrote:
>>> >> > all three applied.
>>> >>
>>> >> Nope, dropped patch 2 again
>>> >
>>> > And dropped the other two also - with them the sequence of two tests I
>>> > was looking at never completes.
>>> >
>>> err... sorry about that.
>>> i'll look into it.
>>
>> I think there's probably two bugs - the test suite should wait for all
>> the DFS stuff to finish.
>>
>> But the fact that it never completes now is strange, maybe somehow radar
>> detection doesn't get turned off properly after your patches?
>>
> it's probably local->radar_detect_enabled doesn't get cleared.
> patch (3) fixes a similar issue, but there seems to be another one.
> (it seems that patch (1) exposed these issues, so i should probably
> reorder the patches and make it the last one)
>
ok, sent a new patchset.

there were some missing updates of local->radar_detect_enabled, so i
preferred to simply remove it altogether and check for radar detection
directly when needed (i.e. on scan/roc).

ap_ht_40mhz_intolerant_ap still seems to fail here, but it fails even
when running only this test, without any of my patches applied.
now i get the same error with my patches applied ("Exception: AP did
not move to 40 MHz channel") instead of the EBUSY one.

i'll try debugging this issue as well.

Eliad.
--
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. 7, 2015, 4:32 p.m. UTC | #8
On Wed, 2015-01-07 at 17:58 +0200, Eliad Peller wrote:

> ok, sent a new patchset.
> 
> there were some missing updates of local->radar_detect_enabled, so i
> preferred to simply remove it altogether and check for radar detection
> directly when needed (i.e. on scan/roc).

Sounds good, thanks.

> ap_ht_40mhz_intolerant_ap still seems to fail here, but it fails even
> when running only this test, without any of my patches applied.
> now i get the same error with my patches applied ("Exception: AP did
> not move to 40 MHz channel") instead of the EBUSY one.
> 
> i'll try debugging this issue as well.

Well, if you want to :)
It's not related to your patches I think, and I saw it passing w/o your
patches applied.

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
Eliad Peller Jan. 7, 2015, 4:42 p.m. UTC | #9
On Wed, Jan 7, 2015 at 6:32 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Wed, 2015-01-07 at 17:58 +0200, Eliad Peller wrote:
>> ap_ht_40mhz_intolerant_ap still seems to fail here, but it fails even
>> when running only this test, without any of my patches applied.
>> now i get the same error with my patches applied ("Exception: AP did
>> not move to 40 MHz channel") instead of the EBUSY one.
>>
>> i'll try debugging this issue as well.
>
> Well, if you want to :)
> It's not related to your patches I think, and I saw it passing w/o your
> patches applied.
>
thanks.
looks like something bad in my setup.
i recompiled hostapd and it seems to pass now :)

Eliad.
--
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 5d6dae9..7f83451 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -388,22 +388,31 @@  ieee80211_find_chanctx(struct ieee80211_local *local,
 	return NULL;
 }
 
-static bool ieee80211_is_radar_required(struct ieee80211_local *local)
+static bool ieee80211_is_radar_required(struct ieee80211_local *local,
+					struct ieee80211_chanctx *ctx)
 {
+	struct ieee80211_chanctx_conf *conf = &ctx->conf;
 	struct ieee80211_sub_if_data *sdata;
+	bool required = false;
 
+	lockdep_assert_held(&local->chanctx_mtx);
 	lockdep_assert_held(&local->mtx);
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(sdata, &local->interfaces, list) {
-		if (sdata->radar_required) {
-			rcu_read_unlock();
-			return true;
-		}
+		if (!ieee80211_sdata_running(sdata))
+			continue;
+		if (rcu_access_pointer(sdata->vif.chanctx_conf) != conf)
+			continue;
+		if (!sdata->radar_required)
+			continue;
+
+		required = true;
+		break;
 	}
 	rcu_read_unlock();
 
-	return false;
+	return required;
 }
 
 static struct ieee80211_chanctx *
@@ -425,7 +434,7 @@  ieee80211_alloc_chanctx(struct ieee80211_local *local,
 	ctx->conf.rx_chains_static = 1;
 	ctx->conf.rx_chains_dynamic = 1;
 	ctx->mode = mode;
-	ctx->conf.radar_enabled = ieee80211_is_radar_required(local);
+	ctx->conf.radar_enabled = ieee80211_is_radar_required(local, ctx);
 	ieee80211_recalc_chanctx_min_def(local, ctx);
 
 	return ctx;
@@ -570,7 +579,7 @@  static void ieee80211_recalc_radar_chanctx(struct ieee80211_local *local,
 	/* for setting local->radar_detect_enabled */
 	lockdep_assert_held(&local->mtx);
 
-	radar_enabled = ieee80211_is_radar_required(local);
+	radar_enabled = ieee80211_is_radar_required(local, chanctx);
 
 	if (radar_enabled == chanctx->conf.radar_enabled)
 		return;