* [RFC] mac80211: correctly close cancelled scans
@ 2013-09-17 5:48 Emmanuel Grumbach
2013-09-17 11:22 ` Eliad Peller
2013-09-30 14:30 ` Johannes Berg
0 siblings, 2 replies; 5+ messages in thread
From: Emmanuel Grumbach @ 2013-09-17 5:48 UTC (permalink / raw)
To: linux-wireless; +Cc: Emmanuel Grumbach
__ieee80211_scan_completed is called from a worker. This
means that the following flow is possible.
* driver calls ieee80211_scan_completed
* mac80211 cancels the scan (that is already complete)
* __ieee80211_scan_complete runs
When scan_work will finally run, it will see that the scan
hasn't been aborted and might even trigger another scan on
another band. This leads to a situation where cfg80211's
scan is not done and no further scan can be issued.
Fix this by setting a new flag when a HW scan is being
cancelled so that no other scan will be triggered.
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
net/mac80211/ieee80211_i.h | 3 +++
net/mac80211/scan.c | 22 +++++++++++++++++++++-
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index d604e16..a523e5a 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -893,6 +893,8 @@ struct tpt_led_trigger {
* that the scan completed.
* @SCAN_ABORTED: Set for our scan work function when the driver reported
* a scan complete for an aborted scan.
+ * @SCAN_HW_CANCELLED: Set for our scan work function when the scan is being
+ * cancelled.
*/
enum {
SCAN_SW_SCANNING,
@@ -900,6 +902,7 @@ enum {
SCAN_ONCHANNEL_SCANNING,
SCAN_COMPLETED,
SCAN_ABORTED,
+ SCAN_HW_CANCELLED,
};
/**
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 08afe74..67f81a4 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -238,6 +238,9 @@ static bool ieee80211_prep_hw_scan(struct ieee80211_local *local)
enum ieee80211_band band;
int i, ielen, n_chans;
+ if (test_bit(SCAN_HW_CANCELLED, &local->scanning))
+ return false;
+
do {
if (local->hw_scan_band == IEEE80211_NUM_BANDS)
return false;
@@ -940,11 +943,28 @@ void ieee80211_scan_cancel(struct ieee80211_local *local)
if (!local->scan_req)
goto out;
+ /*
+ * We have a scan running and the driver already reported completion,
+ * but the worker hasn't run yet or is stuck on the mutex - mark it as
+ * cancelled.
+ */
+ if (test_bit(SCAN_HW_SCANNING, &local->scanning) &&
+ test_bit(SCAN_COMPLETED, &local->scanning)) {
+ set_bit(SCAN_HW_CANCELLED, &local->scanning);
+ goto out;
+ }
+
if (test_bit(SCAN_HW_SCANNING, &local->scanning)) {
- if (local->ops->cancel_hw_scan)
+ /*
+ * Make sure that __ieee80211_scan_completed doesn't trigger a
+ * scan on another band.
+ */
+ set_bit(SCAN_HW_CANCELLED, &local->scanning);
+ if (local->ops->cancel_hw_scan) {
drv_cancel_hw_scan(local,
rcu_dereference_protected(local->scan_sdata,
lockdep_is_held(&local->mtx)));
+ }
goto out;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [RFC] mac80211: correctly close cancelled scans
2013-09-17 5:48 [RFC] mac80211: correctly close cancelled scans Emmanuel Grumbach
@ 2013-09-17 11:22 ` Eliad Peller
2013-09-17 11:28 ` Grumbach, Emmanuel
2013-09-30 14:30 ` Johannes Berg
1 sibling, 1 reply; 5+ messages in thread
From: Eliad Peller @ 2013-09-17 11:22 UTC (permalink / raw)
To: Emmanuel Grumbach; +Cc: linux-wireless@vger.kernel.org
On Tue, Sep 17, 2013 at 8:48 AM, Emmanuel Grumbach
<emmanuel.grumbach@intel.com> wrote:
> __ieee80211_scan_completed is called from a worker. This
> means that the following flow is possible.
>
> * driver calls ieee80211_scan_completed
> * mac80211 cancels the scan (that is already complete)
> * __ieee80211_scan_complete runs
>
> When scan_work will finally run, it will see that the scan
> hasn't been aborted and might even trigger another scan on
> another band. This leads to a situation where cfg80211's
> scan is not done and no further scan can be issued.
>
> Fix this by setting a new flag when a HW scan is being
> cancelled so that no other scan will be triggered.
>
> Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> ---
[...]
> if (test_bit(SCAN_HW_SCANNING, &local->scanning)) {
> - if (local->ops->cancel_hw_scan)
> + /*
> + * Make sure that __ieee80211_scan_completed doesn't trigger a
> + * scan on another band.
> + */
> + set_bit(SCAN_HW_CANCELLED, &local->scanning);
> + if (local->ops->cancel_hw_scan) {
> drv_cancel_hw_scan(local,
> rcu_dereference_protected(local->scan_sdata,
> lockdep_is_held(&local->mtx)));
> + }
> goto out;
> }
you don't seem to clear this flag anywhere...
Eliad.
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [RFC] mac80211: correctly close cancelled scans
2013-09-17 11:22 ` Eliad Peller
@ 2013-09-17 11:28 ` Grumbach, Emmanuel
2013-09-17 11:30 ` Eliad Peller
0 siblings, 1 reply; 5+ messages in thread
From: Grumbach, Emmanuel @ 2013-09-17 11:28 UTC (permalink / raw)
To: Eliad Peller; +Cc: linux-wireless@vger.kernel.org
> On Tue, Sep 17, 2013 at 8:48 AM, Emmanuel Grumbach
> <emmanuel.grumbach@intel.com> wrote:
> > __ieee80211_scan_completed is called from a worker. This means that
> > the following flow is possible.
> >
> > * driver calls ieee80211_scan_completed
> > * mac80211 cancels the scan (that is already complete)
> > * __ieee80211_scan_complete runs
> >
> > When scan_work will finally run, it will see that the scan hasn't been
> > aborted and might even trigger another scan on another band. This
> > leads to a situation where cfg80211's scan is not done and no further
> > scan can be issued.
> >
> > Fix this by setting a new flag when a HW scan is being cancelled so
> > that no other scan will be triggered.
> >
> > Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> > ---
> [...]
>
> > if (test_bit(SCAN_HW_SCANNING, &local->scanning)) {
> > - if (local->ops->cancel_hw_scan)
> > + /*
> > + * Make sure that __ieee80211_scan_completed doesn't trigger a
> > + * scan on another band.
> > + */
> > + set_bit(SCAN_HW_CANCELLED, &local->scanning);
> > + if (local->ops->cancel_hw_scan) {
> > drv_cancel_hw_scan(local,
> > rcu_dereference_protected(local->scan_sdata,
> >
> > lockdep_is_held(&local->mtx)));
> > + }
> > goto out;
> > }
>
> you don't seem to clear this flag anywhere...
>
Yeah - just like SCAN_HW_SCANNING isn't cleared anywhere... but... in __ieee80211_scan_completed:
local->scanning = 0;
local->scan_chandef.chan = NULL;
yes I know. Don't ask.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [RFC] mac80211: correctly close cancelled scans
2013-09-17 11:28 ` Grumbach, Emmanuel
@ 2013-09-17 11:30 ` Eliad Peller
0 siblings, 0 replies; 5+ messages in thread
From: Eliad Peller @ 2013-09-17 11:30 UTC (permalink / raw)
To: Grumbach, Emmanuel; +Cc: linux-wireless@vger.kernel.org
On Tue, Sep 17, 2013 at 2:28 PM, Grumbach, Emmanuel
<emmanuel.grumbach@intel.com> wrote:
>> On Tue, Sep 17, 2013 at 8:48 AM, Emmanuel Grumbach
>> <emmanuel.grumbach@intel.com> wrote:
>> > __ieee80211_scan_completed is called from a worker. This means that
>> > the following flow is possible.
>> >
>> > * driver calls ieee80211_scan_completed
>> > * mac80211 cancels the scan (that is already complete)
>> > * __ieee80211_scan_complete runs
>> >
>> > When scan_work will finally run, it will see that the scan hasn't been
>> > aborted and might even trigger another scan on another band. This
>> > leads to a situation where cfg80211's scan is not done and no further
>> > scan can be issued.
>> >
>> > Fix this by setting a new flag when a HW scan is being cancelled so
>> > that no other scan will be triggered.
>> >
>> > Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
>> > ---
>> [...]
>>
>> > if (test_bit(SCAN_HW_SCANNING, &local->scanning)) {
>> > - if (local->ops->cancel_hw_scan)
>> > + /*
>> > + * Make sure that __ieee80211_scan_completed doesn't trigger a
>> > + * scan on another band.
>> > + */
>> > + set_bit(SCAN_HW_CANCELLED, &local->scanning);
>> > + if (local->ops->cancel_hw_scan) {
>> > drv_cancel_hw_scan(local,
>> > rcu_dereference_protected(local->scan_sdata,
>> >
>> > lockdep_is_held(&local->mtx)));
>> > + }
>> > goto out;
>> > }
>>
>> you don't seem to clear this flag anywhere...
>>
>
> Yeah - just like SCAN_HW_SCANNING isn't cleared anywhere... but... in __ieee80211_scan_completed:
>
> local->scanning = 0;
> local->scan_chandef.chan = NULL;
>
> yes I know. Don't ask.
oh, right, i overlooked it :)
Eliad.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] mac80211: correctly close cancelled scans
2013-09-17 5:48 [RFC] mac80211: correctly close cancelled scans Emmanuel Grumbach
2013-09-17 11:22 ` Eliad Peller
@ 2013-09-30 14:30 ` Johannes Berg
1 sibling, 0 replies; 5+ messages in thread
From: Johannes Berg @ 2013-09-30 14:30 UTC (permalink / raw)
To: Emmanuel Grumbach; +Cc: linux-wireless
On Tue, 2013-09-17 at 08:48 +0300, Emmanuel Grumbach wrote:
> __ieee80211_scan_completed is called from a worker. This
> means that the following flow is possible.
>
> * driver calls ieee80211_scan_completed
> * mac80211 cancels the scan (that is already complete)
> * __ieee80211_scan_complete runs
>
> When scan_work will finally run, it will see that the scan
> hasn't been aborted and might even trigger another scan on
> another band. This leads to a situation where cfg80211's
> scan is not done and no further scan can be issued.
>
> Fix this by setting a new flag when a HW scan is being
> cancelled so that no other scan will be triggered.
I'll apply this
> + if (local->ops->cancel_hw_scan) {
> drv_cancel_hw_scan(local,
> rcu_dereference_protected(local->scan_sdata,
> lockdep_is_held(&local->mtx)));
> + }
WIthout the extra braces.
johannes
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-09-30 14:30 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-17 5:48 [RFC] mac80211: correctly close cancelled scans Emmanuel Grumbach
2013-09-17 11:22 ` Eliad Peller
2013-09-17 11:28 ` Grumbach, Emmanuel
2013-09-17 11:30 ` Eliad Peller
2013-09-30 14:30 ` Johannes Berg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox