From: Luciano Coelho <[email protected]>
Hi,
So, here's my second RFC for the scheduled scan implementation (which we used
to call periodic scan earlier). I have applied all Johannes's comments, except
for two things:
* I kept the return value in the sched_scan_stop chain, because, at least with
wl12xx, the call can fail (due to OOM for instance). I think it's cleaner
this way.
* I have not implemented support for passing the scan interval from userspace
yet. I'll work on it next week and send it as a separate patch.
One of the main things you'll notice is that I merged the patches into two
patches instead of 15, which caused some people to whip me before. :P
Please give me some comments, if you have the time (and bother).
Cheers,
Luca.
Luciano Coelho (2):
cfg80211/nl80211: add support for scheduled scans
mac80211: add support for HW scheduled scan
include/linux/nl80211.h | 9 ++
include/net/cfg80211.h | 48 ++++++++
include/net/mac80211.h | 40 +++++++
net/mac80211/cfg.c | 26 ++++
net/mac80211/driver-ops.h | 30 +++++
net/mac80211/driver-trace.h | 63 ++++++++++
net/mac80211/ieee80211_i.h | 8 ++
net/mac80211/rx.c | 4 +-
net/mac80211/scan.c | 80 +++++++++++++
net/wireless/core.c | 6 +
net/wireless/core.h | 3 +
net/wireless/nl80211.c | 271 +++++++++++++++++++++++++++++++++++++++++++
net/wireless/nl80211.h | 6 +
net/wireless/scan.c | 18 +++
14 files changed, 611 insertions(+), 1 deletions(-)
On Fri, 2010-12-10 at 20:53 +0100, Johannes Berg wrote:
> On Fri, 2010-12-10 at 17:07 +0200, [email protected] wrote:
>
> > * I kept the return value in the sched_scan_stop chain, because, at least with
> > wl12xx, the call can fail (due to OOM for instance). I think it's cleaner
> > this way.
>
> What's going to happen then though? Would it make sense to pre-allocate
> this at start() time, so it can cleanly stop regardless of what's going
> on? I can see start() failing, but stop() failing seems a bit hard to
> work with in wpa_supplicant?
Actually so the nl80211 interface has to be able to return something
like "no such operation in progress" or whatever, but I'm not sure about
the driver interface -- -ENOMEM seems like a stupid failure for stopping
something, and then the above applies ...
johannes
From: Luciano Coelho <[email protected]>
Implement support for HW scheduled scan. The mac80211 code doesn't perform
scheduled scans itself, but calls the driver to start and stop scheduled
scans.
Signed-off-by: Luciano Coelho <[email protected]>
---
include/net/mac80211.h | 40 +++++++++++++++++++++
net/mac80211/cfg.c | 26 ++++++++++++++
net/mac80211/driver-ops.h | 30 ++++++++++++++++
net/mac80211/driver-trace.h | 63 +++++++++++++++++++++++++++++++++
net/mac80211/ieee80211_i.h | 8 ++++
net/mac80211/rx.c | 4 ++-
net/mac80211/scan.c | 80 +++++++++++++++++++++++++++++++++++++++++++
7 files changed, 250 insertions(+), 1 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index e411cf8..2378ca9 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -528,6 +528,21 @@ struct ieee80211_tx_info {
};
};
+/**
+ * ieee80211_sched_scan_ies - scheduled scan IEs
+ *
+ * This structure is used to pass the appropriate IEs to be used in scheduled
+ * scans for all bands. It contains both the IEs passed from the userspace
+ * and the ones generated by mac80211.
+ *
+ * @ie: array with the IEs for each supported band
+ * @len: array with the total length of the IEs for each band
+ */
+struct ieee80211_sched_scan_ies {
+ u8 *ie[IEEE80211_NUM_BANDS];
+ size_t len[IEEE80211_NUM_BANDS];
+};
+
static inline struct ieee80211_tx_info *IEEE80211_SKB_CB(struct sk_buff *skb)
{
return (struct ieee80211_tx_info *)skb->cb;
@@ -1650,6 +1665,15 @@ enum ieee80211_ampdu_mlme_action {
* any error unless this callback returned a negative error code.
* The callback can sleep.
*
+ * @sched_scan_start: Ask the hardware to start scanning repeatedly at
+ * specific intervals. The driver must call the
+ * ieee80211_sched_scan_results() function whenever it finds results.
+ * This process will continue until sched_scan_stop is called.
+ *
+ * @sched_scan_stop: Tell the hardware to stop an ongoing periodic scan.
+ *
+ * ieee80211_sched_scan_results() each time it finds some results.
+ *
* @sw_scan_start: Notifier function that is called just before a software scan
* is started. Can be NULL, if the driver doesn't need this notification.
* The callback can sleep.
@@ -1787,6 +1811,12 @@ struct ieee80211_ops {
u32 iv32, u16 *phase1key);
int (*hw_scan)(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
struct cfg80211_scan_request *req);
+ int (*sched_scan_start)(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif,
+ struct cfg80211_sched_scan_request *req,
+ struct ieee80211_sched_scan_ies *ies);
+ int (*sched_scan_stop)(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif);
void (*sw_scan_start)(struct ieee80211_hw *hw);
void (*sw_scan_complete)(struct ieee80211_hw *hw);
int (*get_stats)(struct ieee80211_hw *hw,
@@ -2369,6 +2399,16 @@ void ieee80211_wake_queues(struct ieee80211_hw *hw);
void ieee80211_scan_completed(struct ieee80211_hw *hw, bool aborted);
/**
+ * ieee80211_sched_scan_results - got results from periodic scan
+ *
+ * When a periodic scan is running, this function needs to be called by the
+ * driver whenever there are new scan results availble.
+ *
+ * @hw: the hardware that is performing periodic scans
+ */
+void ieee80211_sched_scan_results(struct ieee80211_hw *hw);
+
+/**
* ieee80211_iterate_active_interfaces - iterate active interfaces
*
* This function iterates over the interfaces associated with a given
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index c30b8b7..f870c9e 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1255,6 +1255,30 @@ static int ieee80211_scan(struct wiphy *wiphy,
return ieee80211_request_scan(sdata, req);
}
+static int
+ieee80211_sched_scan_start(struct wiphy *wiphy,
+ struct net_device *dev,
+ struct cfg80211_sched_scan_request *req)
+{
+ struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+
+ if (!sdata->local->ops->sched_scan_start)
+ return -EOPNOTSUPP;
+
+ return ieee80211_request_sched_scan_start(sdata, req);
+}
+
+static int
+ieee80211_sched_scan_stop(struct wiphy *wiphy, struct net_device *dev)
+{
+ struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+
+ if (!sdata->local->ops->sched_scan_stop)
+ return -EOPNOTSUPP;
+
+ return ieee80211_request_sched_scan_stop(sdata);
+}
+
static int ieee80211_auth(struct wiphy *wiphy, struct net_device *dev,
struct cfg80211_auth_request *req)
{
@@ -1797,6 +1821,8 @@ struct cfg80211_ops mac80211_config_ops = {
.suspend = ieee80211_suspend,
.resume = ieee80211_resume,
.scan = ieee80211_scan,
+ .sched_scan_start = ieee80211_sched_scan_start,
+ .sched_scan_stop = ieee80211_sched_scan_stop,
.auth = ieee80211_auth,
.assoc = ieee80211_assoc,
.deauth = ieee80211_deauth,
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 4244554..83f2963 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -191,6 +191,36 @@ static inline int drv_hw_scan(struct ieee80211_local *local,
return ret;
}
+static inline int
+drv_sched_scan_start(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata,
+ struct cfg80211_sched_scan_request *req,
+ struct ieee80211_sched_scan_ies *ies)
+{
+ int ret;
+
+ might_sleep();
+
+ trace_drv_sched_scan_start(local, sdata, req);
+ ret = local->ops->sched_scan_start(&local->hw, &sdata->vif,
+ req, ies);
+ trace_drv_return_int(local, ret);
+ return ret;
+}
+
+static inline int drv_sched_scan_stop(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata)
+{
+ int ret;
+
+ might_sleep();
+
+ trace_drv_sched_scan_stop(local, sdata);
+ ret = local->ops->sched_scan_stop(&local->hw, &sdata->vif);
+ trace_drv_return_int(local, ret);
+ return ret;
+}
+
static inline void drv_sw_scan_start(struct ieee80211_local *local)
{
might_sleep();
diff --git a/net/mac80211/driver-trace.h b/net/mac80211/driver-trace.h
index c2772f2..3b19d2c 100644
--- a/net/mac80211/driver-trace.h
+++ b/net/mac80211/driver-trace.h
@@ -439,6 +439,51 @@ TRACE_EVENT(drv_hw_scan,
)
);
+TRACE_EVENT(drv_sched_scan_start,
+ TP_PROTO(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata,
+ struct cfg80211_sched_scan_request *req),
+
+ TP_ARGS(local, sdata, req),
+
+ TP_STRUCT__entry(
+ LOCAL_ENTRY
+ VIF_ENTRY
+ ),
+
+ TP_fast_assign(
+ LOCAL_ASSIGN;
+ VIF_ASSIGN;
+ ),
+
+ TP_printk(
+ LOCAL_PR_FMT VIF_PR_FMT,
+ LOCAL_PR_ARG, VIF_PR_ARG
+ )
+);
+
+TRACE_EVENT(drv_sched_scan_stop,
+ TP_PROTO(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata),
+
+ TP_ARGS(local, sdata),
+
+ TP_STRUCT__entry(
+ LOCAL_ENTRY
+ VIF_ENTRY
+ ),
+
+ TP_fast_assign(
+ LOCAL_ASSIGN;
+ VIF_ASSIGN;
+ ),
+
+ TP_printk(
+ LOCAL_PR_FMT VIF_PR_FMT,
+ LOCAL_PR_ARG, VIF_PR_ARG
+ )
+);
+
TRACE_EVENT(drv_sw_scan_start,
TP_PROTO(struct ieee80211_local *local),
@@ -475,6 +520,24 @@ TRACE_EVENT(drv_sw_scan_complete,
)
);
+TRACE_EVENT(drv_sched_scan_results,
+ TP_PROTO(struct ieee80211_local *local),
+
+ TP_ARGS(local),
+
+ TP_STRUCT__entry(
+ LOCAL_ENTRY
+ ),
+
+ TP_fast_assign(
+ LOCAL_ASSIGN;
+ ),
+
+ TP_printk(
+ LOCAL_PR_FMT, LOCAL_PR_ARG
+ )
+);
+
TRACE_EVENT(drv_get_stats,
TP_PROTO(struct ieee80211_local *local,
struct ieee80211_low_level_stats *stats,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 72499fe..6f726dc 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -642,6 +642,7 @@ enum queue_stop_reason {
* that the scan completed.
* @SCAN_ABORTED: Set for our scan work function when the driver reported
* a scan complete for an aborted scan.
+ * @SCAN_SCHED_SCANNING: We're currently performing periodic scans
*/
enum {
SCAN_SW_SCANNING,
@@ -649,6 +650,7 @@ enum {
SCAN_OFF_CHANNEL,
SCAN_COMPLETED,
SCAN_ABORTED,
+ SCAN_SCHED_SCANNING,
};
/**
@@ -810,6 +812,7 @@ struct ieee80211_local {
enum ieee80211_band hw_scan_band;
int scan_channel_idx;
int scan_ies_len;
+ struct ieee80211_sched_scan_ies sched_scan_ies;
unsigned long leave_oper_channel_time;
enum mac80211_scan_state next_scan_state;
@@ -1109,6 +1112,11 @@ ieee80211_rx_bss_get(struct ieee80211_local *local, u8 *bssid, int freq,
void ieee80211_rx_bss_put(struct ieee80211_local *local,
struct ieee80211_bss *bss);
+/* periodic scan handling */
+int ieee80211_request_sched_scan_start(struct ieee80211_sub_if_data *sdata,
+ struct cfg80211_sched_scan_request *req);
+int ieee80211_request_sched_scan_stop(struct ieee80211_sub_if_data *sdata);
+
/* off-channel helpers */
void ieee80211_offchannel_stop_beaconing(struct ieee80211_local *local);
void ieee80211_offchannel_stop_station(struct ieee80211_local *local);
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 2fe8f5f..b78aba9 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -392,7 +392,8 @@ ieee80211_rx_h_passive_scan(struct ieee80211_rx_data *rx)
if (likely(!(status->rx_flags & IEEE80211_RX_IN_SCAN)))
return RX_CONTINUE;
- if (test_bit(SCAN_HW_SCANNING, &local->scanning))
+ if (test_bit(SCAN_HW_SCANNING, &local->scanning) ||
+ test_bit(SCAN_SCHED_SCANNING, &local->scanning))
return ieee80211_scan_rx(rx->sdata, skb);
if (test_bit(SCAN_SW_SCANNING, &local->scanning)) {
@@ -2713,6 +2714,7 @@ static void __ieee80211_rx_handle_packet(struct ieee80211_hw *hw,
local->dot11ReceivedFragmentCount++;
if (unlikely(test_bit(SCAN_HW_SCANNING, &local->scanning) ||
+ test_bit(SCAN_SCHED_SCANNING, &local->scanning) ||
test_bit(SCAN_OFF_CHANNEL, &local->scanning)))
status->rx_flags |= IEEE80211_RX_IN_SCAN;
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index fb274db..419989f 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -15,6 +15,7 @@
#include <linux/if_arp.h>
#include <linux/rtnetlink.h>
#include <linux/pm_qos_params.h>
+#include <linux/slab.h>
#include <net/sch_generic.h>
#include <linux/slab.h>
#include <net/mac80211.h>
@@ -822,3 +823,82 @@ void ieee80211_scan_cancel(struct ieee80211_local *local)
if (finish)
__ieee80211_scan_completed_finish(&local->hw, false);
}
+
+int ieee80211_request_sched_scan_start(struct ieee80211_sub_if_data *sdata,
+ struct cfg80211_sched_scan_request *req)
+{
+ struct ieee80211_local *local = sdata->local;
+ int ret, i;
+
+ mutex_lock(&sdata->local->mtx);
+
+ if (test_bit(SCAN_SCHED_SCANNING, &local->scanning)) {
+ ret = -EBUSY;
+ goto out;
+ }
+
+ if (!local->ops->sched_scan_start) {
+ ret = -ENOTSUPP;
+ goto out;
+ }
+
+ for (i = 0; i < IEEE80211_NUM_BANDS; i++) {
+ local->sched_scan_ies.ie[i] = kzalloc(2 +
+ IEEE80211_MAX_SSID_LEN +
+ local->scan_ies_len,
+ GFP_KERNEL);
+
+ local->sched_scan_ies.len[i] =
+ ieee80211_build_preq_ies(local,
+ local->sched_scan_ies.ie[i],
+ req->ie, req->ie_len, i,
+ (u32) -1, 0);
+ }
+
+ ret = drv_sched_scan_start(local, sdata, req,
+ &local->sched_scan_ies);
+ if (!ret)
+ __set_bit(SCAN_SCHED_SCANNING, &local->scanning);
+out:
+ mutex_unlock(&sdata->local->mtx);
+
+ return ret;
+}
+
+int ieee80211_request_sched_scan_stop(struct ieee80211_sub_if_data *sdata)
+{
+ struct ieee80211_local *local = sdata->local;
+ int ret = 0, i;
+
+ mutex_lock(&sdata->local->mtx);
+
+ if (!local->ops->sched_scan_stop) {
+ ret = -ENOTSUPP;
+ goto out;
+ }
+
+ if (test_bit(SCAN_SCHED_SCANNING, &local->scanning)) {
+ for (i = 0; i < IEEE80211_NUM_BANDS; i++)
+ kfree(local->sched_scan_ies.ie[i]);
+
+ ret = drv_sched_scan_stop(local, sdata);
+ __clear_bit(SCAN_SCHED_SCANNING, &local->scanning);
+ }
+
+out:
+ mutex_unlock(&sdata->local->mtx);
+
+ return ret;
+}
+
+void ieee80211_sched_scan_results(struct ieee80211_hw *hw)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+
+ mutex_lock(&local->mtx);
+
+ cfg80211_sched_scan_results(hw->wiphy);
+
+ mutex_unlock(&local->mtx);
+}
+EXPORT_SYMBOL(ieee80211_sched_scan_results);
--
1.7.0.4
On Fri, 2010-12-10 at 17:07 +0200, [email protected] wrote:
> Add NL80211_CMD_START_SCHED_SCAN and NL80211_CMD_STOP_SCHED_SCAN
> commands to the nl80211 interface. When results are available they are
> reported by NL80211_CMD_SCHED_SCAN_RESULTS events.
Oh, also, I think you should check the wiphy flag in the commands,
because otherwise somebody will invariably end up writing a driver that
doesn't set the flag, but still expect it to be working :-)
johannes
On Fri, 2010-12-10 at 21:17 +0100, ext Johannes Berg wrote:
> On Fri, 2010-12-10 at 20:53 +0100, Johannes Berg wrote:
> > On Fri, 2010-12-10 at 17:07 +0200, [email protected] wrote:
> >
> > > * I kept the return value in the sched_scan_stop chain, because, at least with
> > > wl12xx, the call can fail (due to OOM for instance). I think it's cleaner
> > > this way.
> >
> > What's going to happen then though? Would it make sense to pre-allocate
> > this at start() time, so it can cleanly stop regardless of what's going
> > on? I can see start() failing, but stop() failing seems a bit hard to
> > work with in wpa_supplicant?
>
> Actually so the nl80211 interface has to be able to return something
> like "no such operation in progress" or whatever, but I'm not sure about
> the driver interface -- -ENOMEM seems like a stupid failure for stopping
> something, and then the above applies ...
Indeed returning -ENOMEM sounds stupid when trying to clean up
something...
In wl12xx there are two other ways the operation can fail: the stop
command can fail, and in this case we start a hw recovery, so there's no
problem; the stop command can time out, which is not handled. Maybe the
timeout should also trigger a hw recovery, but that's a different story.
So, the only thing remaining is the OOM. I guess I'll just have to deal
with it in the driver. We don't pre-alloc any commands at the moment,
but maybe it will have to be done in this case... Let's see.
I'll change the code so that the driver sched_scan stop interface
returns void.
Regarding the "no such operation in progress", I actually handle that by
simply returning with no error. Do you think I should return an error
in that case instead?
--
Cheers,
Luca.
On Fri, 2010-12-10 at 21:05 +0100, ext Johannes Berg wrote:
> On Fri, 2010-12-10 at 17:07 +0200, [email protected] wrote:
>
> > Add NL80211_CMD_START_SCHED_SCAN and NL80211_CMD_STOP_SCHED_SCAN
> > commands to the nl80211 interface. When results are available they are
> > reported by NL80211_CMD_SCHED_SCAN_RESULTS events.
>
> Oh, also, I think you should check the wiphy flag in the commands,
> because otherwise somebody will invariably end up writing a driver that
> doesn't set the flag, but still expect it to be working :-)
Done. It's a good idea to be extra-careful with driver implementations.
--
Cheers,
Luca.
On Fri, 2010-12-10 at 17:07 +0200, [email protected] wrote:
> * I kept the return value in the sched_scan_stop chain, because, at least with
> wl12xx, the call can fail (due to OOM for instance). I think it's cleaner
> this way.
What's going to happen then though? Would it make sense to pre-allocate
this at start() time, so it can cleanly stop regardless of what's going
on? I can see start() failing, but stop() failing seems a bit hard to
work with in wpa_supplicant?
johannes
From: Luciano Coelho <[email protected]>
Implement new functionality for scheduled scan offload. With this feature we
can scan automatically for specific SSIDs (or any if not specified) at
certain intervals.
The idea is that the hardware can perform scan automatically and filter on
desired results without waking up the host unnecessarily.
Add NL80211_CMD_START_SCHED_SCAN and NL80211_CMD_STOP_SCHED_SCAN
commands to the nl80211 interface. When results are available they are
reported by NL80211_CMD_SCHED_SCAN_RESULTS events.
Signed-off-by: Luciano Coelho <[email protected]>
---
include/linux/nl80211.h | 9 ++
include/net/cfg80211.h | 48 +++++++++
net/wireless/core.c | 6 +
net/wireless/core.h | 3 +
net/wireless/nl80211.c | 271 +++++++++++++++++++++++++++++++++++++++++++++++
net/wireless/nl80211.h | 6 +
net/wireless/scan.c | 18 +++
7 files changed, 361 insertions(+), 0 deletions(-)
diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 3804212..4571a90 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -199,6 +199,11 @@
* @NL80211_CMD_SCAN_ABORTED: scan was aborted, for unspecified reasons,
* partial scan results may be available
*
+ * @NL80211_CMD_START_SCHED_SCAN: start a periodic scan
+ * @NL80211_CMD_STOP_SCHED_SCAN: stop a periodic scan
+ * @NL80211_CMD_SCHED_SCAN_RESULTS: there are periodic scan results
+ * available.
+ *
* @NL80211_CMD_GET_SURVEY: get survey resuls, e.g. channel occupation
* or noise level
* @NL80211_CMD_NEW_SURVEY_RESULTS: survey data notification (as a reply to
@@ -508,6 +513,10 @@ enum nl80211_commands {
NL80211_CMD_JOIN_MESH,
NL80211_CMD_LEAVE_MESH,
+ NL80211_CMD_START_SCHED_SCAN,
+ NL80211_CMD_STOP_SCHED_SCAN,
+ NL80211_CMD_SCHED_SCAN_RESULTS,
+
/* add new commands above here */
/* used to define NL80211_CMD_MAX below */
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 0d59799..ff2e8fe 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -741,6 +741,33 @@ struct cfg80211_scan_request {
};
/**
+ * struct cfg80211_sched_scan_request - periodic scan request description
+ *
+ * @ssids: SSIDs to report (other SSIDs will be filtered out)
+ * @n_ssids: number of SSIDs
+ * @n_channels: total number of channels to scan
+ * @ie: optional information element(s) to add into Probe Request or %NULL
+ * @ie_len: length of ie in octets
+ * @wiphy: the wiphy this was for
+ * @dev: the interface
+ * @channels: channels to scan
+ */
+struct cfg80211_sched_scan_request {
+ struct cfg80211_ssid *ssids;
+ int n_ssids;
+ u32 n_channels;
+ const u8 *ie;
+ size_t ie_len;
+
+ /* internal */
+ struct wiphy *wiphy;
+ struct net_device *dev;
+
+ /* keep last */
+ struct ieee80211_channel *channels[0];
+};
+
+/**
* enum cfg80211_signal_type - signal type
*
* @CFG80211_SIGNAL_TYPE_NONE: no signal strength information available
@@ -1173,6 +1200,8 @@ struct cfg80211_pmksa {
* @set_power_mgmt: Configure WLAN power management. A timeout value of -1
* allows the driver to adjust the dynamic ps timeout value.
* @set_cqm_rssi_config: Configure connection quality monitor RSSI threshold.
+ * @sched_scan_start: Tell the driver to start a periodic scan.
+ * @sched_scan_stop: Tell the driver to stop an ongoing periodic scan.
*
* @mgmt_frame_register: Notify driver that a management frame type was
* registered. Note that this callback may not sleep, and cannot run
@@ -1351,6 +1380,12 @@ struct cfg80211_ops {
int (*set_antenna)(struct wiphy *wiphy, u32 tx_ant, u32 rx_ant);
int (*get_antenna)(struct wiphy *wiphy, u32 *tx_ant, u32 *rx_ant);
+
+ int (*sched_scan_start)(struct wiphy *wiphy,
+ struct net_device *dev,
+ struct cfg80211_sched_scan_request *request);
+ int (*sched_scan_stop)(struct wiphy *wiphy,
+ struct net_device *dev);
};
/*
@@ -1393,6 +1428,7 @@ struct cfg80211_ops {
* control port protocol ethertype. The device also honours the
* control_port_no_encrypt flag.
* @WIPHY_FLAG_IBSS_RSN: The device supports IBSS RSN.
+ * @WIPHY_FLAG_SCHED_SCAN: The device supports scheduled scans
*/
enum wiphy_flags {
WIPHY_FLAG_CUSTOM_REGULATORY = BIT(0),
@@ -1404,6 +1440,7 @@ enum wiphy_flags {
WIPHY_FLAG_4ADDR_STATION = BIT(6),
WIPHY_FLAG_CONTROL_PORT_PROTOCOL = BIT(7),
WIPHY_FLAG_IBSS_RSN = BIT(8),
+ WIPHY_FLAG_SUPPORTS_SCHED_SCAN = BIT(9),
};
struct mac_address {
@@ -1457,6 +1494,8 @@ struct ieee80211_txrx_stypes {
* @max_scan_ie_len: maximum length of user-controlled IEs device can
* add to probe request frames transmitted during a scan, must not
* include fixed IEs like supported rates
+ * @max_sched_scan_ssids: maximum number of SSIDs the device can use in
+ * periodic scans
* @coverage_class: current coverage class
* @fw_version: firmware version for ethtool reporting
* @hw_version: hardware version for ethtool reporting
@@ -1493,6 +1532,8 @@ struct wiphy {
u8 max_scan_ssids;
u16 max_scan_ie_len;
+ u8 max_sched_scan_ssids;
+
int n_cipher_suites;
const u32 *cipher_suites;
@@ -2170,6 +2211,13 @@ int cfg80211_wext_siwpmksa(struct net_device *dev,
void cfg80211_scan_done(struct cfg80211_scan_request *request, bool aborted);
/**
+ * cfg80211_sched_scan_results - notify that new scan results are available
+ *
+ * @request: the corresponding periodic scan request
+ */
+void cfg80211_sched_scan_results(struct wiphy *wiphy);
+
+/**
* cfg80211_inform_bss_frame - inform cfg80211 of a received BSS frame
*
* @wiphy: the wiphy reporting the BSS
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 79772fc..241a106 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -365,6 +365,7 @@ struct wiphy *wiphy_new(const struct cfg80211_ops *ops, int sizeof_priv)
spin_lock_init(&rdev->bss_lock);
INIT_LIST_HEAD(&rdev->bss_list);
INIT_WORK(&rdev->scan_done_wk, __cfg80211_scan_done);
+ INIT_WORK(&rdev->sched_scan_wk, __cfg80211_sched_scan_results);
#ifdef CONFIG_CFG80211_WEXT
rdev->wiphy.wext = &cfg80211_wext_handler;
@@ -647,6 +648,11 @@ static void wdev_cleanup_work(struct work_struct *work)
___cfg80211_scan_done(rdev, true);
}
+ if (rdev->sched_scan_req &&
+ rdev->sched_scan_req->dev == wdev->netdev) {
+ nl80211_sched_scan_stopped(rdev, wdev->netdev);
+ }
+
cfg80211_unlock_rdev(rdev);
mutex_lock(&rdev->devlist_mtx);
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 743203b..0d35692 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -60,8 +60,10 @@ struct cfg80211_registered_device {
struct rb_root bss_tree;
u32 bss_generation;
struct cfg80211_scan_request *scan_req; /* protected by RTNL */
+ struct cfg80211_sched_scan_request *sched_scan_req;
unsigned long suspend_at;
struct work_struct scan_done_wk;
+ struct work_struct sched_scan_wk;
#ifdef CONFIG_NL80211_TESTMODE
struct genl_info *testmode_info;
@@ -396,6 +398,7 @@ void cfg80211_sme_rx_auth(struct net_device *dev, const u8 *buf, size_t len);
void cfg80211_sme_disassoc(struct net_device *dev, int idx);
void __cfg80211_scan_done(struct work_struct *wk);
void ___cfg80211_scan_done(struct cfg80211_registered_device *rdev, bool leak);
+void __cfg80211_sched_scan_results(struct work_struct *wk);
void cfg80211_upload_connect_keys(struct wireless_dev *wdev);
int cfg80211_change_iface(struct cfg80211_registered_device *rdev,
struct net_device *dev, enum nl80211_iftype ntype,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index c3f80e5..c8331a5 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -683,6 +683,8 @@ static int nl80211_send_wiphy(struct sk_buff *msg, u32 pid, u32 seq, int flags,
}
CMD(set_channel, SET_CHANNEL);
CMD(set_wds_peer, SET_WDS_PEER);
+ if (dev->wiphy.flags & WIPHY_FLAG_SUPPORTS_SCHED_SCAN)
+ CMD(sched_scan_start, START_SCHED_SCAN);
#undef CMD
@@ -3106,6 +3108,200 @@ static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info *info)
return err;
}
+static int nl80211_start_sched_scan(struct sk_buff *skb,
+ struct genl_info *info)
+{
+ struct cfg80211_sched_scan_request *request;
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ struct net_device *dev = info->user_ptr[1];
+ struct cfg80211_ssid *ssid;
+ struct ieee80211_channel *channel;
+ struct nlattr *attr;
+ struct wiphy *wiphy;
+ int err, tmp, n_ssids = 0, n_channels, i;
+ enum ieee80211_band band;
+ size_t ie_len;
+
+ if (!is_valid_ie_attr(info->attrs[NL80211_ATTR_IE]))
+ return -EINVAL;
+
+ printk(KERN_DEBUG "nl80211_start_sched_scan\n");
+
+ if (!rdev->ops->sched_scan_start) {
+ return -EOPNOTSUPP;
+ }
+
+ if (rdev->sched_scan_req) {
+ return -EINPROGRESS;
+ }
+
+ wiphy = &rdev->wiphy;
+
+ if (info->attrs[NL80211_ATTR_SCAN_FREQUENCIES]) {
+ n_channels = validate_scan_freqs(
+ info->attrs[NL80211_ATTR_SCAN_FREQUENCIES]);
+ if (!n_channels)
+ return -EINVAL;
+ } else {
+ n_channels = 0;
+
+ for (band = 0; band < IEEE80211_NUM_BANDS; band++)
+ if (wiphy->bands[band])
+ n_channels += wiphy->bands[band]->n_channels;
+ }
+
+ if (info->attrs[NL80211_ATTR_SCAN_SSIDS])
+ nla_for_each_nested(attr, info->attrs[NL80211_ATTR_SCAN_SSIDS], tmp)
+ n_ssids++;
+
+ if (n_ssids > wiphy->max_sched_scan_ssids)
+ return -EINVAL;
+
+ if (info->attrs[NL80211_ATTR_IE])
+ ie_len = nla_len(info->attrs[NL80211_ATTR_IE]);
+ else
+ ie_len = 0;
+
+ if (ie_len > wiphy->max_scan_ie_len)
+ return -EINVAL;
+
+ request = kzalloc(sizeof(*request)
+ + sizeof(*ssid) * n_ssids
+ + sizeof(channel) * n_channels
+ + ie_len, GFP_KERNEL);
+ if (!request)
+ return -ENOMEM;
+
+ if (n_ssids)
+ request->ssids = (void *)&request->channels[n_channels];
+ request->n_ssids = n_ssids;
+ if (ie_len) {
+ if (request->ssids)
+ request->ie = (void *)(request->ssids + n_ssids);
+ else
+ request->ie = (void *)(request->channels + n_channels);
+ }
+
+ i = 0;
+ if (info->attrs[NL80211_ATTR_SCAN_FREQUENCIES]) {
+ /* user specified, bail out if channel not found */
+ nla_for_each_nested(attr, info->attrs[NL80211_ATTR_SCAN_FREQUENCIES], tmp) {
+ struct ieee80211_channel *chan;
+
+ chan = ieee80211_get_channel(wiphy, nla_get_u32(attr));
+
+ if (!chan) {
+ err = -EINVAL;
+ goto out_free;
+ }
+
+ /* ignore disabled channels */
+ if (chan->flags & IEEE80211_CHAN_DISABLED)
+ continue;
+
+ request->channels[i] = chan;
+ i++;
+ }
+ } else {
+ /* all channels */
+ for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
+ int j;
+ if (!wiphy->bands[band])
+ continue;
+ for (j = 0; j < wiphy->bands[band]->n_channels; j++) {
+ struct ieee80211_channel *chan;
+
+ chan = &wiphy->bands[band]->channels[j];
+
+ if (chan->flags & IEEE80211_CHAN_DISABLED)
+ continue;
+
+ request->channels[i] = chan;
+ i++;
+ }
+ }
+ }
+
+ if (!i) {
+ err = -EINVAL;
+ goto out_free;
+ }
+
+ request->n_channels = i;
+
+ i = 0;
+ if (info->attrs[NL80211_ATTR_SCAN_SSIDS]) {
+ nla_for_each_nested(attr, info->attrs[NL80211_ATTR_SCAN_SSIDS], tmp) {
+ if (request->ssids[i].ssid_len > IEEE80211_MAX_SSID_LEN) {
+ err = -EINVAL;
+ goto out_free;
+ }
+ memcpy(request->ssids[i].ssid, nla_data(attr), nla_len(attr));
+ request->ssids[i].ssid_len = nla_len(attr);
+ i++;
+ }
+ }
+
+ if (info->attrs[NL80211_ATTR_IE]) {
+ request->ie_len = nla_len(info->attrs[NL80211_ATTR_IE]);
+ memcpy((void *)request->ie,
+ nla_data(info->attrs[NL80211_ATTR_IE]),
+ request->ie_len);
+ }
+
+ request->dev = dev;
+ request->wiphy = &rdev->wiphy;
+
+ rdev->sched_scan_req = request;
+
+ err = rdev->ops->sched_scan_start(&rdev->wiphy, dev, request);
+ if (!err) {
+ nl80211_send_sched_scan(rdev, dev,
+ NL80211_CMD_START_SCHED_SCAN);
+ dev_hold(dev);
+ } else {
+out_free:
+ kfree(request);
+ rdev->sched_scan_req = NULL;
+ }
+
+ return err;
+}
+
+void nl80211_sched_scan_stopped(struct cfg80211_registered_device *rdev,
+ struct net_device *dev)
+{
+ dev_put(dev);
+ kfree(rdev->sched_scan_req);
+ rdev->sched_scan_req = NULL;
+}
+
+static int nl80211_stop_sched_scan(struct sk_buff *skb,
+ struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ struct net_device *dev = info->user_ptr[1];
+ int err;
+
+ printk(KERN_DEBUG "nl80211_stop_sched_scan\n");
+
+ if (!rdev->ops->sched_scan_stop)
+ return -EOPNOTSUPP;
+
+ if (!rdev->sched_scan_req)
+ return 0;
+
+ err = rdev->ops->sched_scan_stop(&rdev->wiphy, dev);
+ if (err)
+ goto out;
+
+ nl80211_send_sched_scan(rdev, dev, NL80211_CMD_STOP_SCHED_SCAN);
+
+ nl80211_sched_scan_stopped(rdev, dev);
+out:
+ return err;
+}
+
static int nl80211_send_bss(struct sk_buff *msg, u32 pid, u32 seq, int flags,
struct cfg80211_registered_device *rdev,
struct wireless_dev *wdev,
@@ -4876,6 +5072,22 @@ static struct genl_ops nl80211_ops[] = {
.dumpit = nl80211_dump_scan,
},
{
+ .cmd = NL80211_CMD_START_SCHED_SCAN,
+ .doit = nl80211_start_sched_scan,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
+ NL80211_FLAG_NEED_RTNL,
+ },
+ {
+ .cmd = NL80211_CMD_STOP_SCHED_SCAN,
+ .doit = nl80211_stop_sched_scan,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
+ NL80211_FLAG_NEED_RTNL,
+ },
+ {
.cmd = NL80211_CMD_AUTHENTICATE,
.doit = nl80211_authenticate,
.policy = nl80211_policy,
@@ -5185,6 +5397,28 @@ static int nl80211_send_scan_msg(struct sk_buff *msg,
return -EMSGSIZE;
}
+static int
+nl80211_send_sched_scan_msg(struct sk_buff *msg,
+ struct cfg80211_registered_device *rdev,
+ struct net_device *netdev,
+ u32 pid, u32 seq, int flags, u32 cmd)
+{
+ void *hdr;
+
+ hdr = nl80211hdr_put(msg, pid, seq, flags, cmd);
+ if (!hdr)
+ return -1;
+
+ NLA_PUT_U32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx);
+ NLA_PUT_U32(msg, NL80211_ATTR_IFINDEX, netdev->ifindex);
+
+ return genlmsg_end(msg, hdr);
+
+ nla_put_failure:
+ genlmsg_cancel(msg, hdr);
+ return -EMSGSIZE;
+}
+
void nl80211_send_scan_start(struct cfg80211_registered_device *rdev,
struct net_device *netdev)
{
@@ -5242,6 +5476,43 @@ void nl80211_send_scan_aborted(struct cfg80211_registered_device *rdev,
nl80211_scan_mcgrp.id, GFP_KERNEL);
}
+void nl80211_send_sched_scan_results(struct cfg80211_registered_device *rdev,
+ struct net_device *netdev)
+{
+ struct sk_buff *msg;
+
+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!msg)
+ return;
+
+ if (nl80211_send_sched_scan_msg(msg, rdev, netdev, 0, 0, 0,
+ NL80211_CMD_SCHED_SCAN_RESULTS) < 0) {
+ nlmsg_free(msg);
+ return;
+ }
+
+ genlmsg_multicast_netns(wiphy_net(&rdev->wiphy), msg, 0,
+ nl80211_scan_mcgrp.id, GFP_KERNEL);
+}
+
+void nl80211_send_sched_scan(struct cfg80211_registered_device *rdev,
+ struct net_device *netdev, u32 cmd)
+{
+ struct sk_buff *msg;
+
+ msg = nlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
+ if (!msg)
+ return;
+
+ if (nl80211_send_sched_scan_msg(msg, rdev, netdev, 0, 0, 0, cmd) < 0) {
+ nlmsg_free(msg);
+ return;
+ }
+
+ genlmsg_multicast_netns(wiphy_net(&rdev->wiphy), msg, 0,
+ nl80211_scan_mcgrp.id, GFP_KERNEL);
+}
+
/*
* This can happen on global regulatory changes or device specific settings
* based on custom world regulatory domains.
diff --git a/net/wireless/nl80211.h b/net/wireless/nl80211.h
index 16c2f71..9e12758 100644
--- a/net/wireless/nl80211.h
+++ b/net/wireless/nl80211.h
@@ -12,6 +12,10 @@ void nl80211_send_scan_done(struct cfg80211_registered_device *rdev,
struct net_device *netdev);
void nl80211_send_scan_aborted(struct cfg80211_registered_device *rdev,
struct net_device *netdev);
+void nl80211_send_sched_scan(struct cfg80211_registered_device *rdev,
+ struct net_device *netdev, u32 cmd);
+void nl80211_send_sched_scan_results(struct cfg80211_registered_device *rdev,
+ struct net_device *netdev);
void nl80211_send_reg_change_event(struct regulatory_request *request);
void nl80211_send_rx_auth(struct cfg80211_registered_device *rdev,
struct net_device *netdev,
@@ -91,5 +95,7 @@ void
nl80211_send_cqm_pktloss_notify(struct cfg80211_registered_device *rdev,
struct net_device *netdev, const u8 *peer,
u32 num_packets, gfp_t gfp);
+void nl80211_sched_scan_stopped(struct cfg80211_registered_device *rdev,
+ struct net_device *dev);
#endif /* __NET_WIRELESS_NL80211_H */
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 503ebb8..e28101c 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -93,6 +93,24 @@ void cfg80211_scan_done(struct cfg80211_scan_request *request, bool aborted)
}
EXPORT_SYMBOL(cfg80211_scan_done);
+void __cfg80211_sched_scan_results(struct work_struct *wk)
+{
+ struct cfg80211_registered_device *rdev;
+
+ rdev = container_of(wk, struct cfg80211_registered_device,
+ sched_scan_wk);
+
+ cfg80211_lock_rdev(rdev);
+ nl80211_send_sched_scan_results(rdev, rdev->sched_scan_req->dev);
+ cfg80211_unlock_rdev(rdev);
+}
+
+void cfg80211_sched_scan_results(struct wiphy *wiphy)
+{
+ queue_work(cfg80211_wq, &wiphy_to_dev(wiphy)->sched_scan_wk);
+}
+EXPORT_SYMBOL(cfg80211_sched_scan_results);
+
static void bss_release(struct kref *ref)
{
struct cfg80211_internal_bss *bss;
--
1.7.0.4
See ... I see nothing here in main.c that would set the wiphy flag, so
you really should've checked it in cfg8021 :-)
> +struct ieee80211_sched_scan_ies {
> + u8 *ie[IEEE80211_NUM_BANDS];
const?
> + * @sched_scan_start: Ask the hardware to start scanning repeatedly at
> + * specific intervals. The driver must call the
> + * ieee80211_sched_scan_results() function whenever it finds results.
> + * This process will continue until sched_scan_stop is called.
> + *
> + * @sched_scan_stop: Tell the hardware to stop an ongoing periodic scan.
> + *
> + * ieee80211_sched_scan_results() each time it finds some results.
I think that should talk about filtering as well? Maybe a DOC: section
would be good, dunno.
> /**
> + * ieee80211_sched_scan_results - got results from periodic scan
> + *
> + * When a periodic scan is running, this function needs to be called by the
> + * driver whenever there are new scan results availble.
typo: available
> +TRACE_EVENT(drv_sched_scan_results,
> + TP_PROTO(struct ieee80211_local *local),
> +
> + TP_ARGS(local),
> +
> + TP_STRUCT__entry(
> + LOCAL_ENTRY
> + ),
> +
> + TP_fast_assign(
> + LOCAL_ASSIGN;
> + ),
> +
> + TP_printk(
> + LOCAL_PR_FMT, LOCAL_PR_ARG
> + )
> +);
Shouldn't that be in the _api_ section?
> @@ -642,6 +642,7 @@ enum queue_stop_reason {
> * that the scan completed.
> * @SCAN_ABORTED: Set for our scan work function when the driver reported
> * a scan complete for an aborted scan.
> + * @SCAN_SCHED_SCANNING: We're currently performing periodic scans
That reminds me ... can you scan and sched_scan at the same time?
sched_scan while associated? Should these be prohibited, or documented
as being implementation dependent?
Also, how does this interact with IDLE? Obviously, the device won't be
idle with this, but you still want it to be "otherwise" idle, no? Should
we take this out of scanning flags and specify that it must be supported
while the device is told that it's idle by mac80211? Do you expect this
to be stopped before trying to associate?
> @@ -392,7 +392,8 @@ ieee80211_rx_h_passive_scan(struct ieee80211_rx_data *rx)
> if (likely(!(status->rx_flags & IEEE80211_RX_IN_SCAN)))
> return RX_CONTINUE;
>
> - if (test_bit(SCAN_HW_SCANNING, &local->scanning))
> + if (test_bit(SCAN_HW_SCANNING, &local->scanning) ||
> + test_bit(SCAN_SCHED_SCANNING, &local->scanning))
> return ieee80211_scan_rx(rx->sdata, skb);
This won't work while associated...
> + for (i = 0; i < IEEE80211_NUM_BANDS; i++) {
> + local->sched_scan_ies.ie[i] = kzalloc(2 +
> + IEEE80211_MAX_SSID_LEN +
> + local->scan_ies_len,
> + GFP_KERNEL);
Oops ... how about if this allocation fails?
> +void ieee80211_sched_scan_results(struct ieee80211_hw *hw)
> +{
> + struct ieee80211_local *local = hw_to_local(hw);
> +
> + mutex_lock(&local->mtx);
> +
> + cfg80211_sched_scan_results(hw->wiphy);
> +
> + mutex_unlock(&local->mtx);
Does that really need locking? Seems ... pointless since cfg80211 will
have to take care of its locking.
Finally: how does this interact with HW reset? Should it be re-started
if it was ever started?
johannes
On Fri, 2010-12-10 at 21:03 +0100, ext Johannes Berg wrote:
> On Fri, 2010-12-10 at 17:07 +0200, [email protected] wrote:
> > With this feature we
> > can scan automatically for specific SSIDs (or any if not specified) at
> > certain intervals.
>
> I'd hope that "if not specified" actually means a passive scan like in
> normal scanning, and you need to specify the wildcard if you want to
> scan for it?
Good point. Actually, from the cfg/mac80211 point of view, this doesn't
really matter. It's just a matter of documenting it so that all the
drivers do it properly and in the same way.
I removed that part of the comment in the commit log and clarified this
in the NL80211 documentation.
For the wl12xx I need to check how exactly this works. As I understand
it, you can either filter on the specified SSIDs or send directed
probe_reqs (ie. with SSID IEs).
Would it make sense to make passive scans (with no SSID specified) and
still filter per SSID? If yes, we should add a new nested attribute to
NL80211 where we can pass the SSID filters.
Actually, I think having a separate NL80211 attribute for SSID filters
is a good idea. What if I remove this whole filtering thingy for now
and add it with a separate patch later?
> > + u8 max_sched_scan_ssids;
>
> shouldn't this be advertised in nl80211 as well?
I think this should be the same as for normal scans. What I have been
using it for was for the filtering SSIDs. I need to rethink the
filtering concept.
> > @@ -647,6 +648,11 @@ static void wdev_cleanup_work(struct work_struct *work)
> > ___cfg80211_scan_done(rdev, true);
> > }
> >
> > + if (rdev->sched_scan_req &&
> > + rdev->sched_scan_req->dev == wdev->netdev) {
> > + nl80211_sched_scan_stopped(rdev, wdev->netdev);
> > + }
>
> Hmm, are you sure that shouldn't be a warning like the scan case? If the
> driver didn't stop -- maybe this is still going on? I think instead the
> netdev down notifier should actually ask the device to stop the sched
> scan (core.c).
Yes, good point. This should work more like "disconnect", which is an
long term process. I have created __cfg80211_stop_sched_scan() and now
I call that from core.c at netdev down and from
nl80211_stop_sched_scan(). Looks cleaner, thanks!
> > + if (!rdev->ops->sched_scan_start) {
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + if (rdev->sched_scan_req) {
> > + return -EINPROGRESS;
> > + }
>
> bit too many braces for my taste :)
Heh, I was probably in an embracing mood when I wrote this. Removed.
> > + if (ie_len > wiphy->max_scan_ie_len)
> > + return -EINVAL;
>
> So # SSIDs is different, but IE len is the same? Isn't that a bad
> assumption to make?
Yes, you're right. As I wrote above, this was about SSID *filters* not
about SSIDs per scan. I'll remove the wiphy->max_sched_scan_ssids for
now and use max_scan_ssids instead.
> > + request->dev = dev;
> > + request->wiphy = &rdev->wiphy;
> > +
> > + rdev->sched_scan_req = request;
> > +
> > + err = rdev->ops->sched_scan_start(&rdev->wiphy, dev, request);
> > + if (!err) {
> > + nl80211_send_sched_scan(rdev, dev,
> > + NL80211_CMD_START_SCHED_SCAN);
> > + dev_hold(dev);
>
> I don't think you want the dev_hold here. That's a trick I used to warn
> about scans that didn't finish when the interface went down. Here,
> instead, since it's a longer-running process, you should do what I said
> above -- stop the sched scan when the interface is going down.
Right, it's not needed. I've removed it now.
> > + err = rdev->ops->sched_scan_stop(&rdev->wiphy, dev);
> > + if (err)
> > + goto out;
>
> return err; instead? There's no cleanup code at the out label :)
Yes, I'm just used to return at the end of the function whenever
possible. We agreed on using this as a coding style for the wl12xx
driver. And if you look at its code, you can see that we sometimes even
goto out from an if even when there's no code between the if block and
the label. Duh!
Fixed.
> > + nl80211_send_sched_scan(rdev, dev, NL80211_CMD_STOP_SCHED_SCAN);
> > +
> > + nl80211_sched_scan_stopped(rdev, dev);
>
> Shouldn't the former be part of the latter function? And actually,
> you'll want to roll it all into a helper function that you can call from
> the netdev down notifier :)
Sure, done as part of the change I described above.
--
Cheers,
Luca.
On Fri, 2010-12-10 at 17:07 +0200, [email protected] wrote:
> With this feature we
> can scan automatically for specific SSIDs (or any if not specified) at
> certain intervals.
I'd hope that "if not specified" actually means a passive scan like in
normal scanning, and you need to specify the wildcard if you want to
scan for it?
> + u8 max_sched_scan_ssids;
shouldn't this be advertised in nl80211 as well?
> @@ -647,6 +648,11 @@ static void wdev_cleanup_work(struct work_struct *work)
> ___cfg80211_scan_done(rdev, true);
> }
>
> + if (rdev->sched_scan_req &&
> + rdev->sched_scan_req->dev == wdev->netdev) {
> + nl80211_sched_scan_stopped(rdev, wdev->netdev);
> + }
Hmm, are you sure that shouldn't be a warning like the scan case? If the
driver didn't stop -- maybe this is still going on? I think instead the
netdev down notifier should actually ask the device to stop the sched
scan (core.c).
> + if (!rdev->ops->sched_scan_start) {
> + return -EOPNOTSUPP;
> + }
> +
> + if (rdev->sched_scan_req) {
> + return -EINPROGRESS;
> + }
bit too many braces for my taste :)
> + if (ie_len > wiphy->max_scan_ie_len)
> + return -EINVAL;
So # SSIDs is different, but IE len is the same? Isn't that a bad
assumption to make?
> + request->dev = dev;
> + request->wiphy = &rdev->wiphy;
> +
> + rdev->sched_scan_req = request;
> +
> + err = rdev->ops->sched_scan_start(&rdev->wiphy, dev, request);
> + if (!err) {
> + nl80211_send_sched_scan(rdev, dev,
> + NL80211_CMD_START_SCHED_SCAN);
> + dev_hold(dev);
I don't think you want the dev_hold here. That's a trick I used to warn
about scans that didn't finish when the interface went down. Here,
instead, since it's a longer-running process, you should do what I said
above -- stop the sched scan when the interface is going down.
> + err = rdev->ops->sched_scan_stop(&rdev->wiphy, dev);
> + if (err)
> + goto out;
return err; instead? There's no cleanup code at the out label :)
> + nl80211_send_sched_scan(rdev, dev, NL80211_CMD_STOP_SCHED_SCAN);
> +
> + nl80211_sched_scan_stopped(rdev, dev);
Shouldn't the former be part of the latter function? And actually,
you'll want to roll it all into a helper function that you can call from
the netdev down notifier :)
johannes
On Fri, 2010-12-10 at 21:15 +0100, ext Johannes Berg wrote:
> See ... I see nothing here in main.c that would set the wiphy flag, so
> you really should've checked it in cfg8021 :-)
Heheh! Well, as we discussed on IRC, I had set the wiphy flag directly
in the driver code. But as you recommended, I have now changed it so
that mac80211 will check if the driver has an op->sched_scan_start and
set the flag accordingly.
> > +struct ieee80211_sched_scan_ies {
> > + u8 *ie[IEEE80211_NUM_BANDS];
>
> const?
As discussed on IRC, in this case this doesn't need to be const. This
struct belongs to mac80211 and it needs to write to it when preparing
the sched_scan, so no need to const it and cast back to non-const when
assigning values to it. There no need to restrict the access to it from
the driver's point-of-view. In normal cases, the driver should not
write to the IEs, but there's no reason to prevent it from doing it.
> > + * @sched_scan_start: Ask the hardware to start scanning repeatedly at
> > + * specific intervals. The driver must call the
> > + * ieee80211_sched_scan_results() function whenever it finds results.
> > + * This process will continue until sched_scan_stop is called.
> > + *
> > + * @sched_scan_stop: Tell the hardware to stop an ongoing periodic scan.
> > + *
> > + * ieee80211_sched_scan_results() each time it finds some results.
>
> I think that should talk about filtering as well? Maybe a DOC: section
> would be good, dunno.
As discussed in your comments to the previous patch, I decided to leave
the filtering out for now and will add it later with a separate patch,
to keep things simple.
There was this extra "ieee80211_sched_scan_results..." pasted wrongly
there, which I have removed.
> > /**
> > + * ieee80211_sched_scan_results - got results from periodic scan
> > + *
> > + * When a periodic scan is running, this function needs to be called by the
> > + * driver whenever there are new scan results availble.
>
> typo: available
Fixed.
> > +TRACE_EVENT(drv_sched_scan_results,
> > + TP_PROTO(struct ieee80211_local *local),
> > +
> > + TP_ARGS(local),
> > +
> > + TP_STRUCT__entry(
> > + LOCAL_ENTRY
> > + ),
> > +
> > + TP_fast_assign(
> > + LOCAL_ASSIGN;
> > + ),
> > +
> > + TP_printk(
> > + LOCAL_PR_FMT, LOCAL_PR_ARG
> > + )
> > +);
>
> Shouldn't that be in the _api_ section?
Indeed. In fact this trace was not even used anywhere. I'll move it to
the api_ section and call it properly.
> > @@ -642,6 +642,7 @@ enum queue_stop_reason {
> > * that the scan completed.
> > * @SCAN_ABORTED: Set for our scan work function when the driver reported
> > * a scan complete for an aborted scan.
> > + * @SCAN_SCHED_SCANNING: We're currently performing periodic scans
>
> That reminds me ... can you scan and sched_scan at the same time?
> sched_scan while associated? Should these be prohibited, or documented
> as being implementation dependent?
With the wl12xx firmware, you can scan and sched_scan at the same time.
In theory, I haven't tried it very thoroughly. It doesn't support
sched_scan while associated, yet. But I think it's a good feature, eg.
for roaming, and we shouldn't restrict it in the mac80211.
I think the best is to document that it is implementation dependent.
And again, for the record, I'll implement a NL80211_SCHED_SCAN_STOPPED
event that the driver can send to userspace at any time when the
sched_scan is running. By doing so, we allow drivers that need to stop
the scan in certain scenarios (eg. while associating or starting a
one-shot scan) to inform the userspace, which then decides whether to
restart it or not.
Maybe we just mandate that sched_scan must work when idle. In other
cases the driver can always return -EBUSY, for instance if it doesn't
support sched_scan while associated.
> Also, how does this interact with IDLE? Obviously, the device won't be
> idle with this, but you still want it to be "otherwise" idle, no? Should
> we take this out of scanning flags and specify that it must be supported
> while the device is told that it's idle by mac80211? Do you expect this
> to be stopped before trying to associate?
The last question is easy to answer: see above. :)
About idle... I have made the assumption that we will consider
sched_scanning as idle, so mac80211 is not calling config() with
IEEE80211_CONF_CHANGE_IDLE when starting or stopping the sched_scan (it
checks local->scan_data when recalculating idle).
But indeed, now checking it more carefully, I can see that the scanning
flags are checked in many different places. I guess the best thing to
do is to take the sched_scan out of the scanning flags and check case by
case. Some kind of new state... we shouldn't suspend things while
sched_scan is running.
> > @@ -392,7 +392,8 @@ ieee80211_rx_h_passive_scan(struct ieee80211_rx_data *rx)
> > if (likely(!(status->rx_flags & IEEE80211_RX_IN_SCAN)))
> > return RX_CONTINUE;
> >
> > - if (test_bit(SCAN_HW_SCANNING, &local->scanning))
> > + if (test_bit(SCAN_HW_SCANNING, &local->scanning) ||
> > + test_bit(SCAN_SCHED_SCANNING, &local->scanning))
> > return ieee80211_scan_rx(rx->sdata, skb);
>
> This won't work while associated...
Damn, this is getting more complicated than I expected. As discussed,
this would eat all beacons during the entire duration of the sched_scan,
so association would break.
Can I change my mind and just forbid sched_scan while not idle? :D
No seriously, I'll continue thinking aboout how to solve this tomorrow.
> > + for (i = 0; i < IEEE80211_NUM_BANDS; i++) {
> > + local->sched_scan_ies.ie[i] = kzalloc(2 +
> > + IEEE80211_MAX_SSID_LEN +
> > + local->scan_ies_len,
> > + GFP_KERNEL);
>
>
> Oops ... how about if this allocation fails?
Oorgh! Fixed.
> > +void ieee80211_sched_scan_results(struct ieee80211_hw *hw)
> > +{
> > + struct ieee80211_local *local = hw_to_local(hw);
> > +
> > + mutex_lock(&local->mtx);
> > +
> > + cfg80211_sched_scan_results(hw->wiphy);
> > +
> > + mutex_unlock(&local->mtx);
>
> Does that really need locking? Seems ... pointless since cfg80211 will
> have to take care of its locking.
Indeed. Removed the locking here.
> Finally: how does this interact with HW reset? Should it be re-started
> if it was ever started?
As described earlier, we can rely on the NL80211_SCHED_SCAN_STOPPED
event, so the userspace may decided whether to restart the sched_scan or
not.
--
Cheers,
Luca.