Hi,
This is the second version of the hardware scan patch. Comparing with
the first version, it does the following:
1. fix a hw scan bug in ieee80211_rx_bss_info() for setting beacon
supported rates
2. let control frames pass in ieee80211_sta_rx_scan() during hw scan
3. set local->sta_{hw|sw}_scanning type to bool
4. avoid channel setting during hw scan
5. rework ieee80211_scan_completed() to make it symmetric for hw scan
The scan code in mac80211 makes the software scan assumption in various
places. For example, we stop the Tx queue during a software scan so that
all the Tx packets will be queued by the stack. We also drop frames not
related to scan in the software scan process. But these are not true for
hardware scan.
Some wireless hardwares (for example iwl3945/4965) has the ability to
perform the whole scan process by hardware and/or firmware. The hardware
scan is relative powerful in that it tries to maintain normal network
traffic while doing a scan in the background. Some drivers (i.e iwlwifi)
do provide a way to tune the hardware scan parameters (for example if the
STA is associated, what's the max time could the STA leave from the
associated channel, how long the scans get suspended after returning to
the service channel, etc). But basically this is transparent to the
stack. mac80211 should not stop Tx queues or drop Rx packets during a
hardware scan.
This patch resolves the above problem by spliting the current scan
indicator local->sta_scanning into local->sta_sw_scanning and
local->sta_hw_scanning. It then changes the scan related code to be aware
of hardware scan or software scan in various places. With this patch,
iwlwifi performs much better in the scan-while-associated condition and
disable_hw_scan=1 should never be required.
Cc: Johannes Berg <[email protected]>
Cc: Mohamed Abbas <[email protected]>
Cc: Ben Cahill <[email protected]>
Signed-off-by: Zhu Yi <[email protected]>
---
diff --git a/net/mac80211/ieee80211.c b/net/mac80211/ieee80211.c
index e0ee65a..d230419 100644
--- a/net/mac80211/ieee80211.c
+++ b/net/mac80211/ieee80211.c
@@ -328,11 +328,14 @@ static int ieee80211_stop(struct net_device *dev)
synchronize_rcu();
skb_queue_purge(&sdata->u.sta.skb_queue);
- if (!local->ops->hw_scan &&
- local->scan_dev == sdata->dev) {
- local->sta_scanning = 0;
- cancel_delayed_work(&local->scan_work);
+ if (local->scan_dev == sdata->dev) {
+ if (!local->ops->hw_scan) {
+ local->sta_sw_scanning = 0;
+ cancel_delayed_work(&local->scan_work);
+ } else
+ local->sta_hw_scanning = 0;
}
+
flush_workqueue(local->hw.workqueue);
/* fall through */
default:
@@ -500,7 +503,7 @@ int ieee80211_hw_config(struct ieee80211_local *local)
struct ieee80211_channel *chan;
int ret = 0;
- if (local->sta_scanning) {
+ if (local->sta_sw_scanning) {
chan = local->scan_channel;
mode = local->scan_hw_mode;
} else {
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 72e1c93..35829b1 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -470,7 +470,8 @@ struct ieee80211_local {
struct list_head interfaces;
- int sta_scanning;
+ bool sta_sw_scanning;
+ bool sta_hw_scanning;
int scan_channel_idx;
enum { SCAN_SET_CHANNEL, SCAN_SEND_PROBE } scan_state;
unsigned long last_scan_completed;
@@ -749,7 +750,8 @@ int ieee80211_sta_req_scan(struct net_device *dev, u8 *ssid, size_t ssid_len);
void ieee80211_sta_req_auth(struct net_device *dev,
struct ieee80211_if_sta *ifsta);
int ieee80211_sta_scan_results(struct net_device *dev, char *buf, size_t len);
-void ieee80211_sta_rx_scan(struct net_device *dev, struct sk_buff *skb,
+ieee80211_txrx_result ieee80211_sta_rx_scan(struct net_device *dev,
+ struct sk_buff *skb,
struct ieee80211_rx_status *rx_status);
void ieee80211_rx_bss_list_init(struct net_device *dev);
void ieee80211_rx_bss_list_deinit(struct net_device *dev);
diff --git a/net/mac80211/ieee80211_ioctl.c b/net/mac80211/ieee80211_ioctl.c
index 942b9cc..741515a 100644
--- a/net/mac80211/ieee80211_ioctl.c
+++ b/net/mac80211/ieee80211_ioctl.c
@@ -315,7 +315,7 @@ int ieee80211_set_channel(struct ieee80211_local *local, int channel, int freq)
}
if (set) {
- if (local->sta_scanning)
+ if (local->sta_sw_scanning || local->sta_hw_scanning)
ret = 0;
else
ret = ieee80211_hw_config(local);
@@ -558,8 +558,10 @@ static int ieee80211_ioctl_giwscan(struct net_device *dev,
{
int res;
struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
- if (local->sta_scanning)
+
+ if (local->sta_sw_scanning || local->sta_hw_scanning)
return -EAGAIN;
+
res = ieee80211_sta_scan_results(dev, extra, data->length);
if (res >= 0) {
data->length = res;
diff --git a/net/mac80211/ieee80211_sta.c b/net/mac80211/ieee80211_sta.c
index 015b3f8..26f404a 100644
--- a/net/mac80211/ieee80211_sta.c
+++ b/net/mac80211/ieee80211_sta.c
@@ -1487,8 +1487,18 @@ static void ieee80211_rx_bss_info(struct net_device *dev,
u32 supp_rates, prev_rates;
int i, j;
- mode = local->sta_scanning ?
+ mode = local->sta_sw_scanning ?
local->scan_hw_mode : local->oper_hw_mode;
+
+ if (local->sta_hw_scanning) {
+ /* search for the correct mode matches the beacon */
+ list_for_each_entry(mode, &local->modes_list, list)
+ if (mode->mode == rx_status->phymode)
+ break;
+
+ if (mode == NULL)
+ mode = local->oper_hw_mode;
+ }
rates = mode->rates;
num_rates = mode->num_rates;
@@ -1871,31 +1881,39 @@ static void ieee80211_sta_rx_queued_mgmt(struct net_device *dev,
}
-void ieee80211_sta_rx_scan(struct net_device *dev, struct sk_buff *skb,
- struct ieee80211_rx_status *rx_status)
+ieee80211_txrx_result
+ieee80211_sta_rx_scan(struct net_device *dev, struct sk_buff *skb,
+ struct ieee80211_rx_status *rx_status)
{
struct ieee80211_mgmt *mgmt;
u16 fc;
- if (skb->len < 24) {
- dev_kfree_skb(skb);
- return;
- }
+ if (skb->len < 2)
+ return TXRX_DROP;
mgmt = (struct ieee80211_mgmt *) skb->data;
fc = le16_to_cpu(mgmt->frame_control);
+ if ((fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_CTL)
+ return TXRX_CONTINUE;
+
+ if (skb->len < 24)
+ return TXRX_DROP;
+
if ((fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_MGMT) {
if ((fc & IEEE80211_FCTL_STYPE) == IEEE80211_STYPE_PROBE_RESP) {
ieee80211_rx_mgmt_probe_resp(dev, mgmt,
skb->len, rx_status);
+ dev_kfree_skb(skb);
+ return TXRX_QUEUED;
} else if ((fc & IEEE80211_FCTL_STYPE) == IEEE80211_STYPE_BEACON) {
ieee80211_rx_mgmt_beacon(dev, mgmt, skb->len,
rx_status);
+ dev_kfree_skb(skb);
+ return TXRX_QUEUED;
}
}
-
- dev_kfree_skb(skb);
+ return TXRX_CONTINUE;
}
@@ -1985,7 +2003,7 @@ void ieee80211_sta_work(struct work_struct *work)
if (!netif_running(dev))
return;
- if (local->sta_scanning)
+ if (local->sta_sw_scanning || local->sta_hw_scanning)
return;
if (sdata->type != IEEE80211_IF_TYPE_STA &&
@@ -2643,9 +2661,15 @@ void ieee80211_scan_completed(struct ieee80211_hw *hw)
union iwreq_data wrqu;
local->last_scan_completed = jiffies;
- wmb();
- local->sta_scanning = 0;
+ memset(&wrqu, 0, sizeof(wrqu));
+ wireless_send_event(dev, SIOCGIWSCAN, &wrqu, NULL);
+ if (local->sta_hw_scanning) {
+ local->sta_hw_scanning = 0;
+ goto done;
+ }
+
+ local->sta_sw_scanning = 0;
if (ieee80211_hw_config(local))
printk(KERN_DEBUG "%s: failed to restore operational"
"channel after scan\n", dev->name);
@@ -2661,9 +2685,6 @@ void ieee80211_scan_completed(struct ieee80211_hw *hw)
netif_tx_unlock_bh(local->mdev);
- memset(&wrqu, 0, sizeof(wrqu));
- wireless_send_event(dev, SIOCGIWSCAN, &wrqu, NULL);
-
rcu_read_lock();
list_for_each_entry_rcu(sdata, &local->interfaces, list) {
@@ -2681,6 +2702,7 @@ void ieee80211_scan_completed(struct ieee80211_hw *hw)
}
rcu_read_unlock();
+done:
sdata = IEEE80211_DEV_TO_SUB_IF(dev);
if (sdata->type == IEEE80211_IF_TYPE_IBSS) {
struct ieee80211_if_sta *ifsta = &sdata->u.sta;
@@ -2703,7 +2725,7 @@ void ieee80211_sta_scan_work(struct work_struct *work)
int skip;
unsigned long next_delay = 0;
- if (!local->sta_scanning)
+ if (!local->sta_sw_scanning)
return;
switch (local->scan_state) {
@@ -2766,7 +2788,7 @@ void ieee80211_sta_scan_work(struct work_struct *work)
break;
}
- if (local->sta_scanning)
+ if (local->sta_sw_scanning)
queue_delayed_work(local->hw.workqueue, &local->scan_work,
next_delay);
}
@@ -2798,7 +2820,7 @@ static int ieee80211_sta_start_scan(struct net_device *dev,
* ResultCode: SUCCESS, INVALID_PARAMETERS
*/
- if (local->sta_scanning) {
+ if (local->sta_sw_scanning || local->sta_hw_scanning) {
if (local->scan_dev == dev)
return 0;
return -EBUSY;
@@ -2806,15 +2828,15 @@ static int ieee80211_sta_start_scan(struct net_device *dev,
if (local->ops->hw_scan) {
int rc = local->ops->hw_scan(local_to_hw(local),
- ssid, ssid_len);
+ ssid, ssid_len);
if (!rc) {
- local->sta_scanning = 1;
+ local->sta_hw_scanning = 1;
local->scan_dev = dev;
}
return rc;
}
- local->sta_scanning = 1;
+ local->sta_sw_scanning = 1;
rcu_read_lock();
list_for_each_entry_rcu(sdata, &local->interfaces, list) {
@@ -2869,7 +2891,7 @@ int ieee80211_sta_req_scan(struct net_device *dev, u8 *ssid, size_t ssid_len)
if (sdata->type != IEEE80211_IF_TYPE_STA)
return ieee80211_sta_start_scan(dev, ssid, ssid_len);
- if (local->sta_scanning) {
+ if (local->sta_sw_scanning || local->sta_hw_scanning) {
if (local->scan_dev == dev)
return 0;
return -EBUSY;
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 428a9fc..596ff71 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -338,8 +338,14 @@ ieee80211_rx_h_passive_scan(struct ieee80211_txrx_data *rx)
struct ieee80211_local *local = rx->local;
struct sk_buff *skb = rx->skb;
- if (unlikely(local->sta_scanning != 0)) {
- ieee80211_sta_rx_scan(rx->dev, skb, rx->u.rx.status);
+ if (unlikely(local->sta_hw_scanning))
+ return ieee80211_sta_rx_scan(rx->dev, skb, rx->u.rx.status);
+
+ if (unlikely(local->sta_sw_scanning)) {
+ /* drop all the other packets during a software scan anyway */
+ if (ieee80211_sta_rx_scan(rx->dev, skb, rx->u.rx.status)
+ != TXRX_QUEUED)
+ dev_kfree_skb(skb);
return TXRX_QUEUED;
}
@@ -1486,7 +1492,7 @@ void __ieee80211_rx(struct ieee80211_hw *hw, struct sk_buff *skb,
goto end;
}
- if (unlikely(local->sta_scanning))
+ if (unlikely(local->sta_sw_scanning || local->sta_hw_scanning))
rx.flags |= IEEE80211_TXRXD_RXIN_SCAN;
if (__ieee80211_invoke_rx_handlers(local, local->rx_pre_handlers, &rx,
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 1a53154..a6a657f 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -225,7 +225,7 @@ ieee80211_tx_h_check_assoc(struct ieee80211_txrx_data *tx)
if (unlikely(tx->flags & IEEE80211_TXRXD_TX_INJECTED))
return TXRX_CONTINUE;
- if (unlikely(tx->local->sta_scanning != 0) &&
+ if (unlikely(tx->local->sta_sw_scanning) &&
((tx->fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_MGMT ||
(tx->fc & IEEE80211_FCTL_STYPE) != IEEE80211_STYPE_PROBE_REQ))
return TXRX_DROP;
> -----Original Message-----
> From: Johannes Berg [mailto:[email protected]]
> Sent: Tuesday, November 20, 2007 11:30 AM
> To: Cahill, Ben M
> Cc: Zhu, Yi; [email protected];
> [email protected]; Abbas, Mohamed
> Subject: RE: [PATCH] mac80211: hardware scan rework (V2)
>
>
> > The firmware-driven scan on 4965 (3945 also) returns to the
> original
> > channel by itself. [...]
>
> Right. Thanks for the description.
>
> > Once done with the list, the scan engine returns to the service
> > channel automatically. While it *might* work okay to
> change or reset
> > that channel, it is not necessary, and might be asking for trouble.
>
> Well, in the patch right now the channel the user sets will
> be completely ignored because the scan_finished thing doesn't
> reconfigure the hardware as it should.
>
> Since the user has an expectation that the working channel is
> changed after setting it with the ioctl, the code should make
> sure. Since hw scanning is mostly transparent to mac80211,
> the ioctl should imho be passed to the driver right away. If
> there's a problem with setting it to the hardware directly,
> the driver can cache it for until the firmware scan finished,
> but I don't think we should do that in mac80211. It has to be
> done somewhere though and just ignoring the setting is wrong.
I see ... thanks, I didn't quite understand the context. Agreed, if the
user says to do something, we should not throw it on the floor.
-- Ben --
>
> johannes
>
Hi,
Thanks!
> 1. fix a hw scan bug in ieee80211_rx_bss_info() for setting beacon
> supported rates
> 2. let control frames pass in ieee80211_sta_rx_scan() during hw scan
> 3. set local->sta_{hw|sw}_scanning type to bool
> 4. avoid channel setting during hw scan
Why this? I was confused at first and I'm sorry if I confused you, but
since we're now fully unaware of hardware scan thanks to your item 5, I
think we need to allow setting channel during hardware scan so the
firmware will change to the right channel once it finished scanning.
> 5. rework ieee80211_scan_completed() to make it symmetric for hw scan
> diff --git a/net/mac80211/ieee80211_sta.c b/net/mac80211/ieee80211_sta.c
> index 015b3f8..26f404a 100644
> --- a/net/mac80211/ieee80211_sta.c
> +++ b/net/mac80211/ieee80211_sta.c
> @@ -1487,8 +1487,18 @@ static void ieee80211_rx_bss_info(struct net_device *dev,
> u32 supp_rates, prev_rates;
> int i, j;
>
> - mode = local->sta_scanning ?
> + mode = local->sta_sw_scanning ?
> local->scan_hw_mode : local->oper_hw_mode;
> +
> + if (local->sta_hw_scanning) {
> + /* search for the correct mode matches the beacon */
> + list_for_each_entry(mode, &local->modes_list, list)
> + if (mode->mode == rx_status->phymode)
> + break;
> +
> + if (mode == NULL)
> + mode = local->oper_hw_mode;
> + }
Good catch.
> @@ -1985,7 +2003,7 @@ void ieee80211_sta_work(struct work_struct *work)
> if (!netif_running(dev))
> return;
>
> - if (local->sta_scanning)
> + if (local->sta_sw_scanning || local->sta_hw_scanning)
> return;
Shouldn't the sta work be able to run normally while hw scan is in
progress?
> - if (unlikely(local->sta_scanning != 0)) {
> - ieee80211_sta_rx_scan(rx->dev, skb, rx->u.rx.status);
> + if (unlikely(local->sta_hw_scanning))
> + return ieee80211_sta_rx_scan(rx->dev, skb, rx->u.rx.status);
> +
> + if (unlikely(local->sta_sw_scanning)) {
> + /* drop all the other packets during a software scan anyway */
> + if (ieee80211_sta_rx_scan(rx->dev, skb, rx->u.rx.status)
> + != TXRX_QUEUED)
> + dev_kfree_skb(skb);
Not entirely sure why we do this, but nothing we should change with this
patch.
Other than the ioctl thing it looks good to me.
johannes
Johannes
The firmware will go back to the old setting, the ones before scan, once
done scanning by it self. The calls by mac80211 after scan complete to
set channel will be ignored by the iwl driver if we still tuned to the
same channel, iwl wont call firmware on the same setting, it has to be a
change to call down to firmware. I need to check what happen if user
change channel during scan, this call might be necessary in this case.
Mohamed
-----Original Message-----
From: Johannes Berg [mailto:[email protected]]
Sent: Tuesday, November 20, 2007 5:51 AM
To: Zhu, Yi
Cc: [email protected]; [email protected]; Abbas,
Mohamed; Cahill, Ben M
Subject: Re: [PATCH] mac80211: hardware scan rework (V2)
Hi,
Thanks!
> 1. fix a hw scan bug in ieee80211_rx_bss_info() for setting beacon
> supported rates
> 2. let control frames pass in ieee80211_sta_rx_scan() during hw scan
> 3. set local->sta_{hw|sw}_scanning type to bool
> 4. avoid channel setting during hw scan
Why this? I was confused at first and I'm sorry if I confused you, but
since we're now fully unaware of hardware scan thanks to your item 5, I
think we need to allow setting channel during hardware scan so the
firmware will change to the right channel once it finished scanning.
> 5. rework ieee80211_scan_completed() to make it symmetric for hw scan
> diff --git a/net/mac80211/ieee80211_sta.c
b/net/mac80211/ieee80211_sta.c
> index 015b3f8..26f404a 100644
> --- a/net/mac80211/ieee80211_sta.c
> +++ b/net/mac80211/ieee80211_sta.c
> @@ -1487,8 +1487,18 @@ static void ieee80211_rx_bss_info(struct
net_device *dev,
> u32 supp_rates, prev_rates;
> int i, j;
>
> - mode = local->sta_scanning ?
> + mode = local->sta_sw_scanning ?
> local->scan_hw_mode : local->oper_hw_mode;
> +
> + if (local->sta_hw_scanning) {
> + /* search for the correct mode matches the
beacon */
> + list_for_each_entry(mode, &local->modes_list,
list)
> + if (mode->mode == rx_status->phymode)
> + break;
> +
> + if (mode == NULL)
> + mode = local->oper_hw_mode;
> + }
Good catch.
> @@ -1985,7 +2003,7 @@ void ieee80211_sta_work(struct work_struct
*work)
> if (!netif_running(dev))
> return;
>
> - if (local->sta_scanning)
> + if (local->sta_sw_scanning || local->sta_hw_scanning)
> return;
Shouldn't the sta work be able to run normally while hw scan is in
progress?
> - if (unlikely(local->sta_scanning != 0)) {
> - ieee80211_sta_rx_scan(rx->dev, skb, rx->u.rx.status);
> + if (unlikely(local->sta_hw_scanning))
> + return ieee80211_sta_rx_scan(rx->dev, skb,
rx->u.rx.status);
> +
> + if (unlikely(local->sta_sw_scanning)) {
> + /* drop all the other packets during a software scan
anyway */
> + if (ieee80211_sta_rx_scan(rx->dev, skb, rx->u.rx.status)
> + != TXRX_QUEUED)
> + dev_kfree_skb(skb);
Not entirely sure why we do this, but nothing we should change with this
patch.
Other than the ioctl thing it looks good to me.
johannes
> The firmware-driven scan on 4965 (3945 also) returns to the original
> channel by itself. [...]
Right. Thanks for the description.
> Once done with the list, the scan engine returns to the service channel
> automatically. While it *might* work okay to change or reset that
> channel, it is not necessary, and might be asking for trouble.
Well, in the patch right now the channel the user sets will be
completely ignored because the scan_finished thing doesn't reconfigure
the hardware as it should.
Since the user has an expectation that the working channel is changed
after setting it with the ioctl, the code should make sure. Since hw
scanning is mostly transparent to mac80211, the ioctl should imho be
passed to the driver right away. If there's a problem with setting it to
the hardware directly, the driver can cache it for until the firmware
scan finished, but I don't think we should do that in mac80211. It has
to be done somewhere though and just ignoring the setting is wrong.
johannes
On Tue, 2007-11-20 at 12:21 -0500, Dan Williams wrote:
> > Johannes
> > The firmware will go back to the old setting, the ones before scan, once
> > done scanning by it self. The calls by mac80211 after scan complete to
> > set channel will be ignored by the iwl driver if we still tuned to the
> > same channel, iwl wont call firmware on the same setting, it has to be a
> > change to call down to firmware. I need to check what happen if user
> > change channel during scan, this call might be necessary in this case.
>
> I'd expect that either (a) the scan gets canceled and then the channel
> change happens, or (b) the channel change gets queued and happens
> immediately after the scan completes. (b) is probably the easier thing
> to do here unless the ucode has a cancel-scan command, but then you have
> to wait until the scan is actually canceled anyway, which sort of
> implies (b) too. So maybe just do (b) and be done with it :)
mac80211 software scanning will do (b) so I expect the driver to handle
the configuration callback and queue the channel switch to after the
ucode finishes scanning.
johannes
> -----Original Message-----
> From: Johannes Berg [mailto:[email protected]]
> Sent: Tuesday, November 20, 2007 8:51 AM
> To: Zhu, Yi
> Cc: [email protected]; [email protected];
> Abbas, Mohamed; Cahill, Ben M
> Subject: Re: [PATCH] mac80211: hardware scan rework (V2)
>
> Hi,
>
> Thanks!
>
> > 1. fix a hw scan bug in ieee80211_rx_bss_info() for setting beacon
> > supported rates
> > 2. let control frames pass in ieee80211_sta_rx_scan()
> during hw scan
> > 3. set local->sta_{hw|sw}_scanning type to bool 4. avoid channel
> > setting during hw scan
>
> Why this? I was confused at first and I'm sorry if I confused
> you, but since we're now fully unaware of hardware scan
> thanks to your item 5, I think we need to allow setting
> channel during hardware scan so the firmware will change to
> the right channel once it finished scanning.
The firmware-driven scan on 4965 (3945 also) returns to the original
channel by itself. It might even do this several times while scanning a
given list of channels, depending on how the timing is set up. Within
the uCode scan command, the driver sets "max_out_time" to restrict the
scan engine from being away from the service (network) channel longer
than a certain amount of time. The scan engine will work its way
through the channel list until it doesn't have enough time to complete
the next channel's scan, then it returns to the service channel.
Conversely, the "suspend_time" controls how long the scan engine stays
back on the service channel (after returning from a scan channel) before
resuming the scan with the next channel on the list.
If the "max_out_time" is set short, then the scan engine will take
several iterations before it completes the list of scan channels. This
is the mechanism that keeps traffic "flowing" during the course of a
scan.
Once done with the list, the scan engine returns to the service channel
automatically. While it *might* work okay to change or reset that
channel, it is not necessary, and might be asking for trouble.
-- Ben --
>
> > 5. rework ieee80211_scan_completed() to make it symmetric
> for hw scan
>
>
> > diff --git a/net/mac80211/ieee80211_sta.c
> > b/net/mac80211/ieee80211_sta.c index 015b3f8..26f404a 100644
> > --- a/net/mac80211/ieee80211_sta.c
> > +++ b/net/mac80211/ieee80211_sta.c
> > @@ -1487,8 +1487,18 @@ static void
> ieee80211_rx_bss_info(struct net_device *dev,
> > u32 supp_rates, prev_rates;
> > int i, j;
> >
> > - mode = local->sta_scanning ?
> > + mode = local->sta_sw_scanning ?
> > local->scan_hw_mode : local->oper_hw_mode;
> > +
> > + if (local->sta_hw_scanning) {
> > + /* search for the correct mode matches
> the beacon */
> > + list_for_each_entry(mode,
> &local->modes_list, list)
> > + if (mode->mode == rx_status->phymode)
> > + break;
> > +
> > + if (mode == NULL)
> > + mode = local->oper_hw_mode;
> > + }
>
> Good catch.
>
> > @@ -1985,7 +2003,7 @@ void ieee80211_sta_work(struct
> work_struct *work)
> > if (!netif_running(dev))
> > return;
> >
> > - if (local->sta_scanning)
> > + if (local->sta_sw_scanning || local->sta_hw_scanning)
> > return;
>
> Shouldn't the sta work be able to run normally while hw scan
> is in progress?
>
> > - if (unlikely(local->sta_scanning != 0)) {
> > - ieee80211_sta_rx_scan(rx->dev, skb, rx->u.rx.status);
> > + if (unlikely(local->sta_hw_scanning))
> > + return ieee80211_sta_rx_scan(rx->dev, skb,
> rx->u.rx.status);
> > +
> > + if (unlikely(local->sta_sw_scanning)) {
> > + /* drop all the other packets during a software
> scan anyway */
> > + if (ieee80211_sta_rx_scan(rx->dev, skb, rx->u.rx.status)
> > + != TXRX_QUEUED)
> > + dev_kfree_skb(skb);
>
> Not entirely sure why we do this, but nothing we should
> change with this patch.
>
> Other than the ioctl thing it looks good to me.
>
> johannes
>
On Tue, 2007-11-20 at 09:17 -0800, Abbas, Mohamed wrote:
> Johannes
> The firmware will go back to the old setting, the ones before scan, once
> done scanning by it self. The calls by mac80211 after scan complete to
> set channel will be ignored by the iwl driver if we still tuned to the
> same channel, iwl wont call firmware on the same setting, it has to be a
> change to call down to firmware. I need to check what happen if user
> change channel during scan, this call might be necessary in this case.
I'd expect that either (a) the scan gets canceled and then the channel
change happens, or (b) the channel change gets queued and happens
immediately after the scan completes. (b) is probably the easier thing
to do here unless the ucode has a cancel-scan command, but then you have
to wait until the scan is actually canceled anyway, which sort of
implies (b) too. So maybe just do (b) and be done with it :)
Dan
> Mohamed
>
> -----Original Message-----
> From: Johannes Berg [mailto:[email protected]]
> Sent: Tuesday, November 20, 2007 5:51 AM
> To: Zhu, Yi
> Cc: [email protected]; [email protected]; Abbas,
> Mohamed; Cahill, Ben M
> Subject: Re: [PATCH] mac80211: hardware scan rework (V2)
>
> Hi,
>
> Thanks!
>
> > 1. fix a hw scan bug in ieee80211_rx_bss_info() for setting beacon
> > supported rates
> > 2. let control frames pass in ieee80211_sta_rx_scan() during hw scan
> > 3. set local->sta_{hw|sw}_scanning type to bool
> > 4. avoid channel setting during hw scan
>
> Why this? I was confused at first and I'm sorry if I confused you, but
> since we're now fully unaware of hardware scan thanks to your item 5, I
> think we need to allow setting channel during hardware scan so the
> firmware will change to the right channel once it finished scanning.
>
> > 5. rework ieee80211_scan_completed() to make it symmetric for hw scan
>
>
> > diff --git a/net/mac80211/ieee80211_sta.c
> b/net/mac80211/ieee80211_sta.c
> > index 015b3f8..26f404a 100644
> > --- a/net/mac80211/ieee80211_sta.c
> > +++ b/net/mac80211/ieee80211_sta.c
> > @@ -1487,8 +1487,18 @@ static void ieee80211_rx_bss_info(struct
> net_device *dev,
> > u32 supp_rates, prev_rates;
> > int i, j;
> >
> > - mode = local->sta_scanning ?
> > + mode = local->sta_sw_scanning ?
> > local->scan_hw_mode : local->oper_hw_mode;
> > +
> > + if (local->sta_hw_scanning) {
> > + /* search for the correct mode matches the
> beacon */
> > + list_for_each_entry(mode, &local->modes_list,
> list)
> > + if (mode->mode == rx_status->phymode)
> > + break;
> > +
> > + if (mode == NULL)
> > + mode = local->oper_hw_mode;
> > + }
>
> Good catch.
>
> > @@ -1985,7 +2003,7 @@ void ieee80211_sta_work(struct work_struct
> *work)
> > if (!netif_running(dev))
> > return;
> >
> > - if (local->sta_scanning)
> > + if (local->sta_sw_scanning || local->sta_hw_scanning)
> > return;
>
> Shouldn't the sta work be able to run normally while hw scan is in
> progress?
>
> > - if (unlikely(local->sta_scanning != 0)) {
> > - ieee80211_sta_rx_scan(rx->dev, skb, rx->u.rx.status);
> > + if (unlikely(local->sta_hw_scanning))
> > + return ieee80211_sta_rx_scan(rx->dev, skb,
> rx->u.rx.status);
> > +
> > + if (unlikely(local->sta_sw_scanning)) {
> > + /* drop all the other packets during a software scan
> anyway */
> > + if (ieee80211_sta_rx_scan(rx->dev, skb, rx->u.rx.status)
> > + != TXRX_QUEUED)
> > + dev_kfree_skb(skb);
>
> Not entirely sure why we do this, but nothing we should change with this
> patch.
>
> Other than the ioctl thing it looks good to me.
>
> johannes
> -
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Acually it is better to bock any managemnt frame task while scanning,
in this function we might end up calling the driver to tune to a
channel, which will cause to stop scan. I think it is better to keep
the check and return if we are doing any scan. I guess this condition
is added to stabilize the mac80211 and it will do the same for iwl
driver with HW scan.
> >
> > > @@ -1985,7 +2003,7 @@ void ieee80211_sta_work(struct work_struct
> > *work)
> > > if (!netif_running(dev))
> > > return;
> > >
> > > - if (local->sta_scanning)
> > > + if (local->sta_sw_scanning || local->sta_hw_scanning)
> > > return;
> >
> > Shouldn't the sta work be able to run normally while hw scan is in
> > progress?
> >