[RFC 1/2] mac80211: disconnect on suspend in managed mode

[RFC 1/2] mac80211: disconnect on suspend in managed mode

[Stanislaw Gruszka]

If possible that after suspend cfg80211 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 AP during suspend.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 net/mac80211/mlme.c | 58 +++++++++--------------------------------------------
 1 file changed, 10 insertions(+), 48 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 9f6464f..19c926d 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -2060,7 +2060,7 @@ struct sk_buff *ieee80211_ap_probereq_get(struct ieee80211_hw *hw,
 }
 EXPORT_SYMBOL(ieee80211_ap_probereq_get);
 
-static void __ieee80211_disconnect(struct ieee80211_sub_if_data *sdata)
+void ieee80211_disconnect(struct ieee80211_sub_if_data *sdata, u8 reason)
 {
 	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
 	u8 frame_buf[IEEE80211_DEAUTH_FRAME_LEN];
@@ -2071,8 +2071,7 @@ static void __ieee80211_disconnect(struct ieee80211_sub_if_data *sdata)
 		return;
 	}
 
-	ieee80211_set_disassoc(sdata, IEEE80211_STYPE_DEAUTH,
-			       WLAN_REASON_DISASSOC_DUE_TO_INACTIVITY,
+	ieee80211_set_disassoc(sdata, IEEE80211_STYPE_DEAUTH, reason,
 			       true, frame_buf);
 	ifmgd->flags &= ~IEEE80211_STA_CSA_RECEIVED;
 	ieee80211_wake_queues_by_reason(&sdata->local->hw,
@@ -2105,7 +2104,8 @@ static void ieee80211_beacon_connection_loss_work(struct work_struct *work)
 	if (ifmgd->connection_loss) {
 		sdata_info(sdata, "Connection to AP %pM lost\n",
 			   ifmgd->bssid);
-		__ieee80211_disconnect(sdata);
+		ieee80211_disconnect(sdata,
+				     WLAN_REASON_DISASSOC_DUE_TO_INACTIVITY);
 	} else {
 		ieee80211_mgd_probe_ap(sdata, true);
 	}
@@ -2117,7 +2117,7 @@ static void ieee80211_csa_connection_drop_work(struct work_struct *work)
 		container_of(work, struct ieee80211_sub_if_data,
 			     u.mgd.csa_connection_drop_work);
 
-	__ieee80211_disconnect(sdata);
+	ieee80211_disconnect(sdata, WLAN_REASON_DISASSOC_DUE_TO_INACTIVITY);
 }
 
 void ieee80211_beacon_loss(struct ieee80211_vif *vif)
@@ -3502,60 +3502,22 @@ void ieee80211_sta_quiesce(struct ieee80211_sub_if_data *sdata)
 {
 	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
 
+	ieee80211_disconnect(sdata, WLAN_REASON_DISASSOC_STA_HAS_LEFT);
+
 	/*
-	 * 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.
+	 * As we disassociate all timers are canceled and pending works will
+	 * perform no action, but we need to wait until works will finish,
+	 * before we can freeze.
 	 */
-
 	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
 
-- 
1.7.11.7

[RFC 2/2] mac80211: sync suspend and stop interface

[Stanislaw Gruszka]

Is possible that we close interface while we are suspended, that
results in warning like below (and some other 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]

To fix check if interface was removed during suspend in
ieee80211_do_stop() before perform any action that require operation
on the interface.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 net/mac80211/iface.c | 21 ++++++++++++---------
 net/mac80211/pm.c    |  2 ++
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 2c059e5..7703141 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 || local->wowlan)
+		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 || local->wowlan) {
+			/*
+			 * Free all remaining keys, there shouldn't be any,
+			 * except maybe group keys in AP more or WDS?
+			 */
+			ieee80211_free_keys(sdata);
+
+			if (going_down)
+				drv_remove_interface(local, sdata);
+		}
 	}
 
 	sdata->bss = NULL;
diff --git a/net/mac80211/pm.c b/net/mac80211/pm.c
index d0275f3..51b4ec7 100644
--- a/net/mac80211/pm.c
+++ b/net/mac80211/pm.c
@@ -182,6 +182,8 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
 
 			mutex_unlock(&local->chanctx_mtx);
 		}
+
+		ieee80211_free_keys(sdata);
 		drv_remove_interface(local, sdata);
 	}
 
-- 
1.7.11.7

Re: [RFC 1/2] mac80211: disconnect on suspend in managed mode

[Johannes Berg]

On Tue, 2013-02-19 at 17:47 +0100, Stanislaw Gruszka wrote:
> If possible that after suspend cfg80211 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:
[...]
> To fix the problem disconnect from AP during suspend.

This will fix the immediate problem, but not in general. What if we're
joined to an IBSS while suspending? A similar thing could happen. Or
operating as an AP? Or joined to a mesh? etc.

I think it'd be better to implement this as explicit disconnect, leave,
teardown, whatever ... in cfg80211 rather than mac80211, although then
of course mac80211 could have a bunch of cleanups, e.g. the various
_quiesce functions could be reduced then I think.

johannes

Re: [RFC 2/2] mac80211: sync suspend and stop interface

[Johannes Berg]

On Tue, 2013-02-19 at 17:48 +0100, Stanislaw Gruszka wrote:
> Is possible that we close interface while we are suspended, that
> results in warning like below (and some other 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]
> 
> To fix check if interface was removed during suspend in
> ieee80211_do_stop() before perform any action that require operation
> on the interface.

This would also become simpler then because the key stuff for instance
wouldn't be necessary any more.

johannes

Re: [RFC 1/2] mac80211: disconnect on suspend in managed mode

[Stanislaw Gruszka]

On Wed, Feb 20, 2013 at 11:06:44AM +0100, Johannes Berg wrote:
> On Tue, 2013-02-19 at 17:47 +0100, Stanislaw Gruszka wrote:
> > If possible that after suspend cfg80211 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:
> [...]
> > To fix the problem disconnect from AP during suspend.
> 
> This will fix the immediate problem, but not in general. What if we're
> joined to an IBSS while suspending? A similar thing could happen. Or
> operating as an AP? Or joined to a mesh? etc.
> 
> I think it'd be better to implement this as explicit disconnect, leave,
> teardown, whatever ... in cfg80211 rather than mac80211, although then
> of course mac80211 could have a bunch of cleanups, e.g. the various
> _quiesce functions could be reduced then I think.

Ok, I have below patch:

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..64e951f 100644
--- a/net/wireless/sysfs.c
+++ b/net/wireless/sysfs.c
@@ -86,16 +86,21 @@ 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) {
+		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;
 }

Re: [RFC 1/2] mac80211: disconnect on suspend in managed mode

[Johannes Berg]

On Thu, 2013-02-21 at 11:29 +0100, Stanislaw Gruszka wrote:

> --- a/net/wireless/sysfs.c
> +++ b/net/wireless/sysfs.c
> @@ -86,16 +86,21 @@ 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) {
> +		list_for_each_entry_rcu(wdev, &rdev->wdev_list, list)
> +			cfg80211_leave(rdev, wdev);
> +
> +		if (rdev->ops->suspend)
>  			ret = rdev_suspend(rdev);

I think this would break WoWLAN?

johannes