If possible that after suspend, cfg80211 will receive request to
disconnect what require action on interface that was removed during
suspend.
Problem can manifest itself by various warnings similar to below one:
WARNING: at net/mac80211/driver-ops.h:12 ieee80211_bss_info_change_notify+0x2f9/0x300 [mac80211]()
wlan0: Failed check-sdata-in-driver check, flags: 0x4
Call Trace:
[<c043e0b3>] warn_slowpath_fmt+0x33/0x40
[<f83707c9>] ieee80211_bss_info_change_notify+0x2f9/0x300 [mac80211]
[<f83a660a>] ieee80211_recalc_ps_vif+0x2a/0x30 [mac80211]
[<f83a6706>] ieee80211_set_disassoc+0xf6/0x500 [mac80211]
[<f83a9441>] ieee80211_mgd_deauth+0x1f1/0x280 [mac80211]
[<f8381b36>] ieee80211_deauth+0x16/0x20 [mac80211]
[<f8261e70>] cfg80211_mlme_down+0x70/0xc0 [cfg80211]
[<f8264de1>] __cfg80211_disconnect+0x1b1/0x1d0 [cfg80211]
To fix the problem disconnect from any associated network before
suspend. User space is responsible to establish connection again
after resume. This basically need to be done by user space anyway,
because associated stations can go away during suspend (for example
NetworkManager disconnects on suspend and connect on resume by default).
Signed-off-by: Stanislaw Gruszka <[email protected]>
---
net/wireless/core.c | 73 +++++++++++++++++++++++++++++-----------------------
net/wireless/core.h | 3 +++
net/wireless/sysfs.c | 15 ++++++++---
3 files changed, 55 insertions(+), 36 deletions(-)
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 5ffff03..5a4dc09 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -815,6 +815,46 @@ void cfg80211_update_iface_num(struct cfg80211_registered_device *rdev,
rdev->num_running_monitor_ifaces += num;
}
+void cfg80211_leave(struct cfg80211_registered_device *rdev,
+ struct wireless_dev *wdev)
+{
+ struct net_device *dev = wdev->netdev;
+
+ switch (wdev->iftype) {
+ case NL80211_IFTYPE_ADHOC:
+ cfg80211_leave_ibss(rdev, dev, true);
+ break;
+ case NL80211_IFTYPE_P2P_CLIENT:
+ case NL80211_IFTYPE_STATION:
+ mutex_lock(&rdev->sched_scan_mtx);
+ __cfg80211_stop_sched_scan(rdev, false);
+ mutex_unlock(&rdev->sched_scan_mtx);
+
+ wdev_lock(wdev);
+#ifdef CONFIG_CFG80211_WEXT
+ kfree(wdev->wext.ie);
+ wdev->wext.ie = NULL;
+ wdev->wext.ie_len = 0;
+ wdev->wext.connect.auth_type = NL80211_AUTHTYPE_AUTOMATIC;
+#endif
+ __cfg80211_disconnect(rdev, dev,
+ WLAN_REASON_DEAUTH_LEAVING, true);
+ cfg80211_mlme_down(rdev, dev);
+ wdev_unlock(wdev);
+ break;
+ case NL80211_IFTYPE_MESH_POINT:
+ cfg80211_leave_mesh(rdev, dev);
+ break;
+ case NL80211_IFTYPE_AP:
+ cfg80211_stop_ap(rdev, dev);
+ break;
+ default:
+ break;
+ }
+
+ wdev->beacon_interval = 0;
+}
+
static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
unsigned long state,
void *ndev)
@@ -883,38 +923,7 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
dev->priv_flags |= IFF_DONT_BRIDGE;
break;
case NETDEV_GOING_DOWN:
- switch (wdev->iftype) {
- case NL80211_IFTYPE_ADHOC:
- cfg80211_leave_ibss(rdev, dev, true);
- break;
- case NL80211_IFTYPE_P2P_CLIENT:
- case NL80211_IFTYPE_STATION:
- mutex_lock(&rdev->sched_scan_mtx);
- __cfg80211_stop_sched_scan(rdev, false);
- mutex_unlock(&rdev->sched_scan_mtx);
-
- wdev_lock(wdev);
-#ifdef CONFIG_CFG80211_WEXT
- kfree(wdev->wext.ie);
- wdev->wext.ie = NULL;
- wdev->wext.ie_len = 0;
- wdev->wext.connect.auth_type = NL80211_AUTHTYPE_AUTOMATIC;
-#endif
- __cfg80211_disconnect(rdev, dev,
- WLAN_REASON_DEAUTH_LEAVING, true);
- cfg80211_mlme_down(rdev, dev);
- wdev_unlock(wdev);
- break;
- case NL80211_IFTYPE_MESH_POINT:
- cfg80211_leave_mesh(rdev, dev);
- break;
- case NL80211_IFTYPE_AP:
- cfg80211_stop_ap(rdev, dev);
- break;
- default:
- break;
- }
- wdev->beacon_interval = 0;
+ cfg80211_leave(rdev, wdev);
break;
case NETDEV_DOWN:
cfg80211_update_iface_num(rdev, wdev->iftype, -1);
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 3aec0e4..878d204 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -503,6 +503,9 @@ int cfg80211_validate_beacon_int(struct cfg80211_registered_device *rdev,
void cfg80211_update_iface_num(struct cfg80211_registered_device *rdev,
enum nl80211_iftype iftype, int num);
+void cfg80211_leave(struct cfg80211_registered_device *rdev,
+ struct wireless_dev *wdev);
+
#define CFG80211_MAX_NUM_DIFFERENT_CHANNELS 10
#ifdef CONFIG_CFG80211_DEVELOPER_WARNINGS
diff --git a/net/wireless/sysfs.c b/net/wireless/sysfs.c
index 238ee49..cf58a91 100644
--- a/net/wireless/sysfs.c
+++ b/net/wireless/sysfs.c
@@ -86,16 +86,23 @@ static int wiphy_uevent(struct device *dev, struct kobj_uevent_env *env)
static int wiphy_suspend(struct device *dev, pm_message_t state)
{
struct cfg80211_registered_device *rdev = dev_to_rdev(dev);
+ struct wireless_dev *wdev;
int ret = 0;
rdev->suspend_at = get_seconds();
- if (rdev->ops->suspend) {
- rtnl_lock();
- if (rdev->wiphy.registered)
+ rtnl_lock();
+ if (rdev->wiphy.registered) {
+ if (!rdev->wowlan) {
+ list_for_each_entry_rcu(wdev, &rdev->wdev_list, list)
+ cfg80211_leave(rdev, wdev);
+ }
+
+ if (rdev->ops->suspend)
ret = rdev_suspend(rdev);
- rtnl_unlock();
+
}
+ rtnl_unlock();
return ret;
}
--
1.7.11.7
Remove not used any longer suspend/resume code.
Signed-off-by: Stanislaw Gruszka <[email protected]>
---
net/mac80211/ibss.c | 29 +----------------------------
net/mac80211/ieee80211_i.h | 4 ----
2 files changed, 1 insertion(+), 32 deletions(-)
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 40b71df..539d4a1 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -985,36 +985,9 @@ static void ieee80211_ibss_timer(unsigned long data)
{
struct ieee80211_sub_if_data *sdata =
(struct ieee80211_sub_if_data *) data;
- struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
- struct ieee80211_local *local = sdata->local;
-
- if (local->quiescing) {
- ifibss->timer_running = true;
- return;
- }
-
- ieee80211_queue_work(&local->hw, &sdata->work);
-}
-
-#ifdef CONFIG_PM
-void ieee80211_ibss_quiesce(struct ieee80211_sub_if_data *sdata)
-{
- struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
- if (del_timer_sync(&ifibss->timer))
- ifibss->timer_running = true;
-}
-
-void ieee80211_ibss_restart(struct ieee80211_sub_if_data *sdata)
-{
- struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
-
- if (ifibss->timer_running) {
- add_timer(&ifibss->timer);
- ifibss->timer_running = false;
- }
+ ieee80211_queue_work(&sdata->local->hw, &sdata->work);
}
-#endif
void ieee80211_ibss_setup_sdata(struct ieee80211_sub_if_data *sdata)
{
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 5230694..bd13db3 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -489,8 +489,6 @@ struct ieee80211_if_ibss {
u32 basic_rates;
- bool timer_running;
-
bool fixed_bssid;
bool fixed_channel;
bool privacy;
@@ -1295,8 +1293,6 @@ void ieee80211_ibss_rx_no_sta(struct ieee80211_sub_if_data *sdata,
int ieee80211_ibss_join(struct ieee80211_sub_if_data *sdata,
struct cfg80211_ibss_params *params);
int ieee80211_ibss_leave(struct ieee80211_sub_if_data *sdata);
-void ieee80211_ibss_quiesce(struct ieee80211_sub_if_data *sdata);
-void ieee80211_ibss_restart(struct ieee80211_sub_if_data *sdata);
void ieee80211_ibss_work(struct ieee80211_sub_if_data *sdata);
void ieee80211_ibss_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata,
struct sk_buff *skb);
--
1.7.11.7
On Mon, 2013-02-25 at 12:50 +0100, Stanislaw Gruszka wrote:
> +void cfg80211_leave(struct cfg80211_registered_device *rdev,
> + struct wireless_dev *wdev);
missing a space here for indentation :)
> - rtnl_unlock();
> +
> }
That added blank line is spurious.
I was going to fix these but have more concerns on the second patch.
johannes
Since now we disconnect before suspend, various code which save
connection state can now be removed from suspend and resume
procedure. Cleanup on resume side is smaller as ieee80211_reconfig()
is also used for H/W restart.
Signed-off-by: Stanislaw Gruszka <[email protected]>
---
net/mac80211/ieee80211_i.h | 4 ---
net/mac80211/key.c | 14 ----------
net/mac80211/key.h | 1 -
net/mac80211/pm.c | 68 ----------------------------------------------
net/mac80211/util.c | 26 ------------------
5 files changed, 113 deletions(-)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 388580a..cc917a7 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -765,10 +765,6 @@ struct ieee80211_sub_if_data {
} debugfs;
#endif
-#ifdef CONFIG_PM
- struct ieee80211_bss_conf suspend_bss_conf;
-#endif
-
/* must be last, dynamically sized area in this! */
struct ieee80211_vif vif;
};
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index ef252eb..df81b17 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -566,20 +566,6 @@ void ieee80211_iter_keys(struct ieee80211_hw *hw,
}
EXPORT_SYMBOL(ieee80211_iter_keys);
-void ieee80211_disable_keys(struct ieee80211_sub_if_data *sdata)
-{
- struct ieee80211_key *key;
-
- ASSERT_RTNL();
-
- mutex_lock(&sdata->local->key_mtx);
-
- list_for_each_entry(key, &sdata->key_list, list)
- ieee80211_key_disable_hw_accel(key);
-
- mutex_unlock(&sdata->local->key_mtx);
-}
-
void ieee80211_free_keys(struct ieee80211_sub_if_data *sdata)
{
struct ieee80211_key *key, *tmp;
diff --git a/net/mac80211/key.h b/net/mac80211/key.h
index 382dc44..8b03730 100644
--- a/net/mac80211/key.h
+++ b/net/mac80211/key.h
@@ -143,7 +143,6 @@ void ieee80211_set_default_mgmt_key(struct ieee80211_sub_if_data *sdata,
int idx);
void ieee80211_free_keys(struct ieee80211_sub_if_data *sdata);
void ieee80211_enable_keys(struct ieee80211_sub_if_data *sdata);
-void ieee80211_disable_keys(struct ieee80211_sub_if_data *sdata);
#define key_mtx_dereference(local, ref) \
rcu_dereference_protected(ref, lockdep_is_held(&((local)->key_mtx)))
diff --git a/net/mac80211/pm.c b/net/mac80211/pm.c
index d0275f3..58f4297 100644
--- a/net/mac80211/pm.c
+++ b/net/mac80211/pm.c
@@ -6,26 +6,6 @@
#include "driver-ops.h"
#include "led.h"
-/* return value indicates whether the driver should be further notified */
-static void ieee80211_quiesce(struct ieee80211_sub_if_data *sdata)
-{
- switch (sdata->vif.type) {
- case NL80211_IFTYPE_STATION:
- ieee80211_sta_quiesce(sdata);
- break;
- case NL80211_IFTYPE_ADHOC:
- ieee80211_ibss_quiesce(sdata);
- break;
- case NL80211_IFTYPE_MESH_POINT:
- ieee80211_mesh_quiesce(sdata);
- break;
- default:
- break;
- }
-
- cancel_work_sync(&sdata->work);
-}
-
int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
{
struct ieee80211_local *local = hw_to_local(hw);
@@ -95,17 +75,10 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
WARN_ON(err != 1);
local->wowlan = false;
} else {
- list_for_each_entry(sdata, &local->interfaces, list)
- if (ieee80211_sdata_running(sdata))
- ieee80211_quiesce(sdata);
goto suspend;
}
}
- /* disable keys */
- list_for_each_entry(sdata, &local->interfaces, list)
- ieee80211_disable_keys(sdata);
-
/* tear down aggregation sessions and remove STAs */
mutex_lock(&local->sta_mtx);
list_for_each_entry(sta, &local->sta_list, list) {
@@ -117,55 +90,14 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
WARN_ON(drv_sta_state(local, sta->sdata, sta,
state, state - 1));
}
-
- mesh_plink_quiesce(sta);
}
mutex_unlock(&local->sta_mtx);
/* remove all interfaces */
list_for_each_entry(sdata, &local->interfaces, list) {
- static u8 zero_addr[ETH_ALEN] = {};
- u32 changed = 0;
-
if (!ieee80211_sdata_running(sdata))
continue;
- switch (sdata->vif.type) {
- case NL80211_IFTYPE_AP_VLAN:
- case NL80211_IFTYPE_MONITOR:
- /* skip these */
- continue;
- case NL80211_IFTYPE_STATION:
- if (sdata->vif.bss_conf.assoc)
- changed = BSS_CHANGED_ASSOC |
- BSS_CHANGED_BSSID |
- BSS_CHANGED_IDLE;
- break;
- case NL80211_IFTYPE_AP:
- case NL80211_IFTYPE_ADHOC:
- case NL80211_IFTYPE_MESH_POINT:
- if (sdata->vif.bss_conf.enable_beacon)
- changed = BSS_CHANGED_BEACON_ENABLED;
- break;
- default:
- break;
- }
-
- ieee80211_quiesce(sdata);
-
- sdata->suspend_bss_conf = sdata->vif.bss_conf;
- memset(&sdata->vif.bss_conf, 0, sizeof(sdata->vif.bss_conf));
- sdata->vif.bss_conf.idle = true;
- if (sdata->suspend_bss_conf.bssid)
- sdata->vif.bss_conf.bssid = zero_addr;
-
- /* disable beaconing or remove association */
- ieee80211_bss_info_change_notify(sdata, changed);
-
- if (sdata->vif.type == NL80211_IFTYPE_AP &&
- rcu_access_pointer(sdata->u.ap.beacon))
- drv_stop_ap(local, sdata);
-
if (local->use_chanctx) {
struct ieee80211_chanctx_conf *conf;
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 0f38f43..f5d4e32 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1534,11 +1534,6 @@ int ieee80211_reconfig(struct ieee80211_local *local)
BSS_CHANGED_IDLE |
BSS_CHANGED_TXPOWER;
-#ifdef CONFIG_PM
- if (local->resuming && !reconfig_due_to_wowlan)
- sdata->vif.bss_conf = sdata->suspend_bss_conf;
-#endif
-
switch (sdata->vif.type) {
case NL80211_IFTYPE_STATION:
changed |= BSS_CHANGED_ASSOC |
@@ -1678,28 +1673,7 @@ int ieee80211_reconfig(struct ieee80211_local *local)
mb();
local->resuming = false;
- list_for_each_entry(sdata, &local->interfaces, list) {
- switch(sdata->vif.type) {
- case NL80211_IFTYPE_STATION:
- ieee80211_sta_restart(sdata);
- break;
- case NL80211_IFTYPE_ADHOC:
- ieee80211_ibss_restart(sdata);
- break;
- case NL80211_IFTYPE_MESH_POINT:
- ieee80211_mesh_restart(sdata);
- break;
- default:
- break;
- }
- }
-
mod_timer(&local->sta_cleanup, jiffies + 1);
-
- mutex_lock(&local->sta_mtx);
- list_for_each_entry(sta, &local->sta_list, list)
- mesh_plink_restart(sta);
- mutex_unlock(&local->sta_mtx);
#else
WARN_ON(1);
#endif
--
1.7.11.7
Remove not used any longer suspend/resume code.
Signed-off-by: Stanislaw Gruszka <[email protected]>
---
net/mac80211/ieee80211_i.h | 3 --
net/mac80211/mlme.c | 84 ++--------------------------------------------
2 files changed, 2 insertions(+), 85 deletions(-)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index cc917a7..5230694 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -400,7 +400,6 @@ struct ieee80211_if_managed {
u16 aid;
- unsigned long timers_running; /* used for quiesce/restart */
bool powersave; /* powersave requested for this iface */
bool broken_ap; /* AP is broken -- turn off powersave */
u8 dtim_period;
@@ -1279,8 +1278,6 @@ void
ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata,
const struct ieee80211_channel_sw_ie *sw_elem,
struct ieee80211_bss *bss, u64 timestamp);
-void ieee80211_sta_quiesce(struct ieee80211_sub_if_data *sdata);
-void ieee80211_sta_restart(struct ieee80211_sub_if_data *sdata);
void ieee80211_sta_work(struct ieee80211_sub_if_data *sdata);
void ieee80211_sta_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata,
struct sk_buff *skb);
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 9f6464f..410470f 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -87,9 +87,6 @@ MODULE_PARM_DESC(probe_wait_ms,
*/
#define IEEE80211_SIGNAL_AVE_MIN_COUNT 4
-#define TMR_RUNNING_TIMER 0
-#define TMR_RUNNING_CHANSW 1
-
/*
* All cfg80211 functions have to be called outside a locked
* section so that they can acquire a lock themselves... This
@@ -1035,14 +1032,8 @@ static void ieee80211_chswitch_timer(unsigned long data)
{
struct ieee80211_sub_if_data *sdata =
(struct ieee80211_sub_if_data *) data;
- struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
-
- if (sdata->local->quiescing) {
- set_bit(TMR_RUNNING_CHANSW, &ifmgd->timers_running);
- return;
- }
- ieee80211_queue_work(&sdata->local->hw, &ifmgd->chswitch_work);
+ ieee80211_queue_work(&sdata->local->hw, &sdata->u.mgd.chswitch_work);
}
void
@@ -1827,8 +1818,6 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
del_timer_sync(&sdata->u.mgd.timer);
del_timer_sync(&sdata->u.mgd.chswitch_timer);
- sdata->u.mgd.timers_running = 0;
-
sdata->vif.bss_conf.dtim_period = 0;
ifmgd->flags = 0;
@@ -3137,15 +3126,8 @@ static void ieee80211_sta_timer(unsigned long data)
{
struct ieee80211_sub_if_data *sdata =
(struct ieee80211_sub_if_data *) data;
- struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
- struct ieee80211_local *local = sdata->local;
-
- if (local->quiescing) {
- set_bit(TMR_RUNNING_TIMER, &ifmgd->timers_running);
- return;
- }
- ieee80211_queue_work(&local->hw, &sdata->work);
+ ieee80211_queue_work(&sdata->local->hw, &sdata->work);
}
static void ieee80211_sta_connection_lost(struct ieee80211_sub_if_data *sdata,
@@ -3497,68 +3479,6 @@ static void ieee80211_restart_sta_timer(struct ieee80211_sub_if_data *sdata)
}
}
-#ifdef CONFIG_PM
-void ieee80211_sta_quiesce(struct ieee80211_sub_if_data *sdata)
-{
- struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
-
- /*
- * we need to use atomic bitops for the running bits
- * only because both timers might fire at the same
- * time -- the code here is properly synchronised.
- */
-
- cancel_work_sync(&ifmgd->request_smps_work);
-
- cancel_work_sync(&ifmgd->monitor_work);
- cancel_work_sync(&ifmgd->beacon_connection_loss_work);
- cancel_work_sync(&ifmgd->csa_connection_drop_work);
- if (del_timer_sync(&ifmgd->timer))
- set_bit(TMR_RUNNING_TIMER, &ifmgd->timers_running);
-
- cancel_work_sync(&ifmgd->chswitch_work);
- if (del_timer_sync(&ifmgd->chswitch_timer))
- set_bit(TMR_RUNNING_CHANSW, &ifmgd->timers_running);
-
- /* these will just be re-established on connection */
- del_timer_sync(&ifmgd->conn_mon_timer);
- del_timer_sync(&ifmgd->bcn_mon_timer);
-}
-
-void ieee80211_sta_restart(struct ieee80211_sub_if_data *sdata)
-{
- struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
-
- mutex_lock(&ifmgd->mtx);
- if (!ifmgd->associated) {
- mutex_unlock(&ifmgd->mtx);
- return;
- }
-
- if (sdata->flags & IEEE80211_SDATA_DISCONNECT_RESUME) {
- sdata->flags &= ~IEEE80211_SDATA_DISCONNECT_RESUME;
- mlme_dbg(sdata, "driver requested disconnect after resume\n");
- ieee80211_sta_connection_lost(sdata,
- ifmgd->associated->bssid,
- WLAN_REASON_UNSPECIFIED,
- true);
- mutex_unlock(&ifmgd->mtx);
- return;
- }
- mutex_unlock(&ifmgd->mtx);
-
- if (test_and_clear_bit(TMR_RUNNING_TIMER, &ifmgd->timers_running))
- add_timer(&ifmgd->timer);
- if (test_and_clear_bit(TMR_RUNNING_CHANSW, &ifmgd->timers_running))
- add_timer(&ifmgd->chswitch_timer);
- ieee80211_sta_reset_beacon_monitor(sdata);
-
- mutex_lock(&sdata->local->mtx);
- ieee80211_restart_sta_timer(sdata);
- mutex_unlock(&sdata->local->mtx);
-}
-#endif
-
/* interface setup */
void ieee80211_sta_setup_sdata(struct ieee80211_sub_if_data *sdata)
{
--
1.7.11.7
On Tue, Feb 26, 2013 at 09:44:00PM +0100, Johannes Berg wrote:
> More generally, does this really make much sense? There are other things
> here, e.g. ieee80211_configure_filter(), ieee80211_recalc_ps(),
> ieee80211_hw_config() and probably more that can be executed in this
> function -- I don't think protecting these two calls is really
> sufficient?
>
> I think it'd be smarter to delay the down until resumed, or so.
I'm thinking how to do this nicely. For now I'll skip this change
from my patch set.
Stanislaw
On Wed, 2013-02-27 at 11:42 +0100, Stanislaw Gruszka wrote:
> On Tue, Feb 26, 2013 at 09:44:00PM +0100, Johannes Berg wrote:
> > More generally, does this really make much sense? There are other things
> > here, e.g. ieee80211_configure_filter(), ieee80211_recalc_ps(),
> > ieee80211_hw_config() and probably more that can be executed in this
> > function -- I don't think protecting these two calls is really
> > sufficient?
>
> Seems all other drv calls like those on ieee80211_confgure_fitler() do
> not require sdata. So this is most likely sufficient. I'm able to
> reporduce warnings on rt2x00 usb with commit
> 761ce8c41ed20ee3af77f2df527edc3f92e6f3bf reverted. This patch make
> them gone.
Well, we're talking about different things. You're concerned about
warnings, while I'm saying that semantically this shouldn't be called
while the device is stopped as the driver might not expect it. That
might not cause a warning today, but that's only because we didn't put
in a warning like
WARN_ON(!local->device_started);
johannes
On Tue, Feb 26, 2013 at 09:44:00PM +0100, Johannes Berg wrote:
> More generally, does this really make much sense? There are other things
> here, e.g. ieee80211_configure_filter(), ieee80211_recalc_ps(),
> ieee80211_hw_config() and probably more that can be executed in this
> function -- I don't think protecting these two calls is really
> sufficient?
Seems all other drv calls like those on ieee80211_confgure_fitler() do
not require sdata. So this is most likely sufficient. I'm able to
reporduce warnings on rt2x00 usb with commit
761ce8c41ed20ee3af77f2df527edc3f92e6f3bf reverted. This patch make
them gone.
Stanislaw
Remove not used any longer suspend/resume code.
Signed-off-by: Stanislaw Gruszka <[email protected]>
---
net/mac80211/ieee80211_i.h | 2 --
net/mac80211/mesh.c | 57 ++--------------------------------------------
net/mac80211/mesh.h | 12 ----------
net/mac80211/mesh_plink.c | 27 +---------------------
net/mac80211/sta_info.h | 2 --
5 files changed, 3 insertions(+), 97 deletions(-)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index bd13db3..6f3258a 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -540,8 +540,6 @@ struct ieee80211_if_mesh {
struct timer_list mesh_path_timer;
struct timer_list mesh_path_root_timer;
- unsigned long timers_running;
-
unsigned long wrkq_flags;
u8 mesh_id[IEEE80211_MAX_MESH_ID_LEN];
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 29ce2aa..f5d1afa 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -13,10 +13,6 @@
#include "ieee80211_i.h"
#include "mesh.h"
-#define TMR_RUNNING_HK 0
-#define TMR_RUNNING_MP 1
-#define TMR_RUNNING_MPR 2
-
static int mesh_allocated;
static struct kmem_cache *rm_cache;
@@ -50,11 +46,6 @@ static void ieee80211_mesh_housekeeping_timer(unsigned long data)
set_bit(MESH_WORK_HOUSEKEEPING, &ifmsh->wrkq_flags);
- if (local->quiescing) {
- set_bit(TMR_RUNNING_HK, &ifmsh->timers_running);
- return;
- }
-
ieee80211_queue_work(&local->hw, &sdata->work);
}
@@ -479,15 +470,8 @@ static void ieee80211_mesh_path_timer(unsigned long data)
{
struct ieee80211_sub_if_data *sdata =
(struct ieee80211_sub_if_data *) data;
- struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
- struct ieee80211_local *local = sdata->local;
-
- if (local->quiescing) {
- set_bit(TMR_RUNNING_MP, &ifmsh->timers_running);
- return;
- }
- ieee80211_queue_work(&local->hw, &sdata->work);
+ ieee80211_queue_work(&sdata->local->hw, &sdata->work);
}
static void ieee80211_mesh_path_root_timer(unsigned long data)
@@ -495,16 +479,10 @@ static void ieee80211_mesh_path_root_timer(unsigned long data)
struct ieee80211_sub_if_data *sdata =
(struct ieee80211_sub_if_data *) data;
struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
- struct ieee80211_local *local = sdata->local;
set_bit(MESH_WORK_ROOT, &ifmsh->wrkq_flags);
- if (local->quiescing) {
- set_bit(TMR_RUNNING_MPR, &ifmsh->timers_running);
- return;
- }
-
- ieee80211_queue_work(&local->hw, &sdata->work);
+ ieee80211_queue_work(&sdata->local->hw, &sdata->work);
}
void ieee80211_mesh_root_setup(struct ieee80211_if_mesh *ifmsh)
@@ -622,35 +600,6 @@ static void ieee80211_mesh_rootpath(struct ieee80211_sub_if_data *sdata)
round_jiffies(TU_TO_EXP_TIME(interval)));
}
-#ifdef CONFIG_PM
-void ieee80211_mesh_quiesce(struct ieee80211_sub_if_data *sdata)
-{
- struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
-
- /* use atomic bitops in case all timers fire at the same time */
-
- if (del_timer_sync(&ifmsh->housekeeping_timer))
- set_bit(TMR_RUNNING_HK, &ifmsh->timers_running);
- if (del_timer_sync(&ifmsh->mesh_path_timer))
- set_bit(TMR_RUNNING_MP, &ifmsh->timers_running);
- if (del_timer_sync(&ifmsh->mesh_path_root_timer))
- set_bit(TMR_RUNNING_MPR, &ifmsh->timers_running);
-}
-
-void ieee80211_mesh_restart(struct ieee80211_sub_if_data *sdata)
-{
- struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
-
- if (test_and_clear_bit(TMR_RUNNING_HK, &ifmsh->timers_running))
- add_timer(&ifmsh->housekeeping_timer);
- if (test_and_clear_bit(TMR_RUNNING_MP, &ifmsh->timers_running))
- add_timer(&ifmsh->mesh_path_timer);
- if (test_and_clear_bit(TMR_RUNNING_MPR, &ifmsh->timers_running))
- add_timer(&ifmsh->mesh_path_root_timer);
- ieee80211_mesh_root_setup(ifmsh);
-}
-#endif
-
static int
ieee80211_mesh_build_beacon(struct ieee80211_if_mesh *ifmsh)
{
@@ -871,8 +820,6 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)
local->fif_other_bss--;
atomic_dec(&local->iff_allmultis);
ieee80211_configure_filter(local);
-
- sdata->u.mesh.timers_running = 0;
}
static void
diff --git a/net/mac80211/mesh.h b/net/mac80211/mesh.h
index 336c88a..6ffabbe 100644
--- a/net/mac80211/mesh.h
+++ b/net/mac80211/mesh.h
@@ -313,8 +313,6 @@ void mesh_path_timer(unsigned long data);
void mesh_path_flush_by_nexthop(struct sta_info *sta);
void mesh_path_discard_frame(struct ieee80211_sub_if_data *sdata,
struct sk_buff *skb);
-void mesh_path_quiesce(struct ieee80211_sub_if_data *sdata);
-void mesh_path_restart(struct ieee80211_sub_if_data *sdata);
void mesh_path_tx_root_frame(struct ieee80211_sub_if_data *sdata);
bool mesh_action_is_path_sel(struct ieee80211_mgmt *mgmt);
@@ -359,22 +357,12 @@ static inline bool mesh_path_sel_is_hwmp(struct ieee80211_sub_if_data *sdata)
void ieee80211_mesh_notify_scan_completed(struct ieee80211_local *local);
-void ieee80211_mesh_quiesce(struct ieee80211_sub_if_data *sdata);
-void ieee80211_mesh_restart(struct ieee80211_sub_if_data *sdata);
-void mesh_plink_quiesce(struct sta_info *sta);
-void mesh_plink_restart(struct sta_info *sta);
void mesh_path_flush_by_iface(struct ieee80211_sub_if_data *sdata);
void mesh_sync_adjust_tbtt(struct ieee80211_sub_if_data *sdata);
void ieee80211s_stop(void);
#else
static inline void
ieee80211_mesh_notify_scan_completed(struct ieee80211_local *local) {}
-static inline void ieee80211_mesh_quiesce(struct ieee80211_sub_if_data *sdata)
-{}
-static inline void ieee80211_mesh_restart(struct ieee80211_sub_if_data *sdata)
-{}
-static inline void mesh_plink_quiesce(struct sta_info *sta) {}
-static inline void mesh_plink_restart(struct sta_info *sta) {}
static inline bool mesh_path_sel_is_hwmp(struct ieee80211_sub_if_data *sdata)
{ return false; }
static inline void mesh_path_flush_by_iface(struct ieee80211_sub_if_data *sdata)
diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
index 07d396d..08df966 100644
--- a/net/mac80211/mesh_plink.c
+++ b/net/mac80211/mesh_plink.c
@@ -534,10 +534,8 @@ static void mesh_plink_timer(unsigned long data)
*/
sta = (struct sta_info *) data;
- if (sta->sdata->local->quiescing) {
- sta->plink_timer_was_running = true;
+ if (sta->sdata->local->quiescing)
return;
- }
spin_lock_bh(&sta->lock);
if (sta->ignore_plink_timer) {
@@ -598,29 +596,6 @@ static void mesh_plink_timer(unsigned long data)
}
}
-#ifdef CONFIG_PM
-void mesh_plink_quiesce(struct sta_info *sta)
-{
- if (!ieee80211_vif_is_mesh(&sta->sdata->vif))
- return;
-
- /* no kernel mesh sta timers have been initialized */
- if (sta->sdata->u.mesh.security != IEEE80211_MESH_SEC_NONE)
- return;
-
- if (del_timer_sync(&sta->plink_timer))
- sta->plink_timer_was_running = true;
-}
-
-void mesh_plink_restart(struct sta_info *sta)
-{
- if (sta->plink_timer_was_running) {
- add_timer(&sta->plink_timer);
- sta->plink_timer_was_running = false;
- }
-}
-#endif
-
static inline void mesh_plink_timer_set(struct sta_info *sta, int timeout)
{
sta->plink_timer.expires = jiffies + (HZ * timeout / 1000);
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 4947341..e5868c3 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -281,7 +281,6 @@ struct sta_ampdu_mlme {
* @plink_state: peer link state
* @plink_timeout: timeout of peer link
* @plink_timer: peer link watch timer
- * @plink_timer_was_running: used by suspend/resume to restore timers
* @t_offset: timing offset relative to this host
* @t_offset_setpoint: reference timing offset of this sta to be used when
* calculating clockdrift
@@ -379,7 +378,6 @@ struct sta_info {
__le16 reason;
u8 plink_retries;
bool ignore_plink_timer;
- bool plink_timer_was_running;
enum nl80211_plink_state plink_state;
u32 plink_timeout;
struct timer_list plink_timer;
--
1.7.11.7
On Mon, 2013-02-25 at 12:50 +0100, Stanislaw Gruszka wrote:
> @@ -95,17 +75,10 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
> WARN_ON(err != 1);
> local->wowlan = false;
> } else {
Hmmm! The context here reminds me that it is possible to have WoWLAN
configured, but have the driver reject it and then suspend "normally".
Our driver seems to be the only one doing it (both dvm and mvm flavours)
and it does it only in case we aren't connected ...
However overall this patchset will break the functionality, if WoWLAN is
enabled but not possible we won't cleanly disconnect. This is a bit of a
problem, I guess we need to propagate this return value behaviour all
the way into cfg80211 where the cfg80211_leave() can then be called?
johannes
On Mon, 2013-02-25 at 12:50 +0100, Stanislaw Gruszka wrote:
> Is possible that we close interface while we are suspended, that
> results in warning like below (and some others similar):
>
> WARNING: at net/mac80211/driver-ops.h:12 ieee80211_do_stop+0x62e/0x670 [mac80211]()
> wlan0: Failed check-sdata-in-driver check, flags: 0x4
> @@ -834,16 +835,18 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
>
> skb_queue_purge(&sdata->skb_queue);
>
> - /*
> - * Free all remaining keys, there shouldn't be any,
> - * except maybe group keys in AP more or WDS?
> - */
> - ieee80211_free_keys(sdata);
> -
> drv_remove_interface_debugfs(local, sdata);
>
> - if (going_down)
> - drv_remove_interface(local, sdata);
> + if (!local->suspended) {
> + /*
> + * There shouldn't be any kays, sice we disconnected
> + * before suspend.
> + */
> + WARN_ON(!list_empty(&sdata->key_list));
This is wrong -- you're now leaking the keys after a warning; freeing
the keys had nothing to do with suspend originally. You shouldn't change
this, in fact you should just move the comment & code I think.
More generally, does this really make much sense? There are other things
here, e.g. ieee80211_configure_filter(), ieee80211_recalc_ps(),
ieee80211_hw_config() and probably more that can be executed in this
function -- I don't think protecting these two calls is really
sufficient?
I think it'd be smarter to delay the down until resumed, or so.
johannes
Is possible that we close interface while we are suspended, that
results in warning like below (and some others similar):
WARNING: at net/mac80211/driver-ops.h:12 ieee80211_do_stop+0x62e/0x670 [mac80211]()
wlan0: Failed check-sdata-in-driver check, flags: 0x4
Call Trace:
[<ffffffff8105c9d6>] warn_slowpath_fmt+0x46/0x50
[<ffffffffa045d46e>] ieee80211_do_stop+0x62e/0x670 [mac80211]
[<ffffffffa045d4ca>] ieee80211_stop+0x1a/0x20 [mac80211]
[<ffffffff815122ed>] __dev_close_many+0x7d/0xc0
[<ffffffff81513af8>] dev_close_many+0x88/0x100
[<ffffffff81513f2a>] dev_close+0x3a/0x50
[<ffffffffa03c90ae>] cfg80211_rfkill_set_block+0x6e/0xa0 [cfg80211]
[<ffffffffa03c9106>] cfg80211_rfkill_sync_work+0x26/0x30 [cfg80211]
Check for local->suspended in ieee80211_do_stop() before perform
any action that require operation on interface that was removed
from the driver.
All keys should be freed, so remove ieee80211_free_keys() and
add corresponding WARN_ON().
Signed-off-by: Stanislaw Gruszka <[email protected]>
---
net/mac80211/iface.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 2c059e5..370b86b 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -419,7 +419,8 @@ static void ieee80211_del_virtual_monitor(struct ieee80211_local *local)
ieee80211_vif_release_channel(sdata);
- drv_remove_interface(local, sdata);
+ if (!local->suspended)
+ drv_remove_interface(local, sdata);
kfree(sdata);
out_unlock:
@@ -834,16 +835,18 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
skb_queue_purge(&sdata->skb_queue);
- /*
- * Free all remaining keys, there shouldn't be any,
- * except maybe group keys in AP more or WDS?
- */
- ieee80211_free_keys(sdata);
-
drv_remove_interface_debugfs(local, sdata);
- if (going_down)
- drv_remove_interface(local, sdata);
+ if (!local->suspended) {
+ /*
+ * There shouldn't be any kays, sice we disconnected
+ * before suspend.
+ */
+ WARN_ON(!list_empty(&sdata->key_list));
+
+ if (going_down)
+ drv_remove_interface(local, sdata);
+ }
}
sdata->bss = NULL;
--
1.7.11.7
On Wed, Feb 27, 2013 at 03:54:10PM +0100, Stanislaw Gruszka wrote:
> On Tue, Feb 26, 2013 at 09:44:00PM +0100, Johannes Berg wrote:
> > More generally, does this really make much sense? There are other things
> > here, e.g. ieee80211_configure_filter(), ieee80211_recalc_ps(),
> > ieee80211_hw_config() and probably more that can be executed in this
> > function -- I don't think protecting these two calls is really
> > sufficient?
> >
> > I think it'd be smarter to delay the down until resumed, or so.
>
> I'm thinking how to do this nicely. For now I'll skip this change
> from my patch set.
Unfortunately delay down after resume approach has problem, if driver
was removed during suspend, ieee8011_do_stop will never happen and we
leak resources.
I changed patch to do more calls conditionally if down during suspend,
some work is still needed. For example I don't know how handle roc,
add_virtual_monitor and channel context stuff.
Anyway I hope we could apply something like below, so some warnings
we have reported will get fix.
Stanislaw
---
net/mac80211/iface.c | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index dfe9cb9..7a0d9ce 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -419,7 +419,8 @@ static void ieee80211_del_virtual_monitor(struct ieee80211_local *local)
ieee80211_vif_release_channel(sdata);
- drv_remove_interface(local, sdata);
+ if (!local->suspended)
+ drv_remove_interface(local, sdata);
kfree(sdata);
out_unlock:
@@ -744,8 +745,7 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
sdata->dev->addr_len);
spin_unlock_bh(&local->filter_lock);
netif_addr_unlock_bh(sdata->dev);
-
- ieee80211_configure_filter(local);
+ /* configure filter latter (if not suspended) */
}
del_timer_sync(&local->dynamic_ps_timer);
@@ -810,10 +810,7 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
}
ieee80211_adjust_monitor_flags(sdata, -1);
- ieee80211_configure_filter(local);
- mutex_lock(&local->mtx);
- ieee80211_recalc_idle(local);
- mutex_unlock(&local->mtx);
+ /* tell driver latter (if not suspended) */
break;
case NL80211_IFTYPE_P2P_DEVICE:
/* relies on synchronize_rcu() below */
@@ -844,26 +841,29 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
case NL80211_IFTYPE_AP:
skb_queue_purge(&sdata->skb_queue);
- if (going_down)
+ if (going_down && !local->suspended)
drv_remove_interface(local, sdata);
}
sdata->bss = NULL;
- ieee80211_recalc_ps(local, -1);
+ if (!local->suspended) {
+ if (local->open_count == 0) {
+ ieee80211_clear_tx_pending(local);
+ ieee80211_stop_device(local);
+ } else {
+ ieee80211_configure_filter(local);
+ ieee80211_recalc_ps(local, -1);
- if (local->open_count == 0) {
- ieee80211_clear_tx_pending(local);
- ieee80211_stop_device(local);
+ mutex_lock(&local->mtx);
+ ieee80211_recalc_idle(local);
+ mutex_unlock(&local->mtx);
- /* no reconfiguring after stop! */
- hw_reconf_flags = 0;
+ if (hw_reconf_flags)
+ ieee80211_hw_config(local, hw_reconf_flags);
+ }
}
- /* do after stop to avoid reconfiguring when we stop anyway */
- if (hw_reconf_flags)
- ieee80211_hw_config(local, hw_reconf_flags);
-
spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
for (i = 0; i < IEEE80211_MAX_QUEUES; i++) {
skb_queue_walk_safe(&local->pending[i], skb, tmp) {
--
1.7.11.7