Re: [RFC] mac80211: deauthenticate STAs before suspend

Re: [RFC] mac80211: deauthenticate STAs before suspend

[Arend van Spriel]

>
> IOW, I'd rather leave it as is. It seems there isn't really a problem,
> but rather a perceived inconsistency that is being "fixed"?

Agree. It may be considered a fix from a particular user-space application
perspective (ie. NetworkManager) but from standards perspective there is
no reason to do this.

>
> johannes
>

Gr. AvS

Re: [RFC] mac80211: deauthenticate STAs before suspend

[Rajkumar Manoharan]

On Tue, Feb 01, 2011 at 03:32:28PM +0530, Arend van Spriel wrote:
> >
> > IOW, I'd rather leave it as is. It seems there isn't really a problem,
> > but rather a perceived inconsistency that is being "fixed"?
> 
> Agree. It may be considered a fix from a particular user-space application
> perspective (ie. NetworkManager) but from standards perspective there is
> no reason to do this.
> 
I have seen this issue as the AP resources are locked for a while, if STA
is not resume back sooner.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC] mac80211: deauthenticate STAs before suspend

[Rajkumar Manoharan]

The STA is not deauthenticated before the system goes to sleep,
when the vif was configured by iw/iwconfig. This holds the AP
resources unnecessarily, till the STA entry got expired in AP.
So before going to sleep ensure that all stations
got deauthenticated.

Signed-off-by: Rajkumar Manoharan <rmanoharan@atheros.com>
---
 include/net/cfg80211.h |    2 ++
 net/mac80211/cfg.c     |   21 +++++++++++++++++++++
 net/wireless/sysfs.c   |    6 ++++++
 3 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 679a049..c50345f 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1056,6 +1056,7 @@ struct cfg80211_pmksa {
  * wireless extensions but this is subject to reevaluation as soon as this
  * code is used more widely and we have a first user without wext.
  *
+ * @pre_suspend: dissociate STAs before suspending the wiphy device
  * @suspend: wiphy device needs to be suspended
  * @resume: wiphy device needs to be resumed
  *
@@ -1196,6 +1197,7 @@ struct cfg80211_pmksa {
  * @get_antenna: Get current antenna configuration from device (tx_ant, rx_ant).
  */
 struct cfg80211_ops {
+	int	(*pre_suspend)(struct wiphy *wiphy);
 	int	(*suspend)(struct wiphy *wiphy);
 	int	(*resume)(struct wiphy *wiphy);
 
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 4bc8a92..2daadf1 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1245,6 +1245,25 @@ static int ieee80211_set_channel(struct wiphy *wiphy,
 }
 
 #ifdef CONFIG_PM
+static int ieee80211_pre_suspend(struct wiphy *wiphy)
+{
+	struct ieee80211_local *local = wiphy_priv(wiphy);
+	struct ieee80211_sub_if_data *sdata = NULL;
+	struct ieee80211_if_managed *ifmgd = NULL;
+	struct cfg80211_deauth_request req;
+
+	list_for_each_entry(sdata, &local->interfaces, list) {
+		ifmgd = &sdata->u.mgd;
+		if (ifmgd->associated) {
+			req.bss = ifmgd->associated;
+			req.reason_code = WLAN_REASON_DEAUTH_LEAVING;
+			req.local_state_change = false;
+			ieee80211_mgd_deauth(sdata, &req, NULL);
+		}
+	}
+	return 0;
+}
+
 static int ieee80211_suspend(struct wiphy *wiphy)
 {
 	return __ieee80211_suspend(wiphy_priv(wiphy));
@@ -1255,6 +1274,7 @@ static int ieee80211_resume(struct wiphy *wiphy)
 	return __ieee80211_resume(wiphy_priv(wiphy));
 }
 #else
+#define ieee80211_pre_suspend NULL
 #define ieee80211_suspend NULL
 #define ieee80211_resume NULL
 #endif
@@ -1969,6 +1989,7 @@ struct cfg80211_ops mac80211_config_ops = {
 	.change_bss = ieee80211_change_bss,
 	.set_txq_params = ieee80211_set_txq_params,
 	.set_channel = ieee80211_set_channel,
+	.pre_suspend = ieee80211_pre_suspend,
 	.suspend = ieee80211_suspend,
 	.resume = ieee80211_resume,
 	.scan = ieee80211_scan,
diff --git a/net/wireless/sysfs.c b/net/wireless/sysfs.c
index 4294fa2..944d589 100644
--- a/net/wireless/sysfs.c
+++ b/net/wireless/sysfs.c
@@ -91,6 +91,12 @@ static int wiphy_suspend(struct device *dev, pm_message_t state)
 
 	rdev->suspend_at = get_seconds();
 
+	if (rdev->ops->pre_suspend) {
+		rtnl_lock();
+		ret = rdev->ops->pre_suspend(&rdev->wiphy);
+		rtnl_unlock();
+	}
+
 	if (rdev->ops->suspend) {
 		rtnl_lock();
 		ret = rdev->ops->suspend(&rdev->wiphy);
-- 
1.7.3.5

Re: [RFC] mac80211: deauthenticate STAs before suspend

[Johannes Berg]

On Mon, 2011-01-31 at 15:36 +0530, Rajkumar Manoharan wrote:
> The STA is not deauthenticated before the system goes to sleep,
> when the vif was configured by iw/iwconfig. This holds the AP
> resources unnecessarily, till the STA entry got expired in AP.

Is that a big problem? We actually had this intentionally, and we go to
PS before going to sleep, so if the sleep is very short then we will not
be disconnected. Also, it's much easier this way to implement WoWLAN.

> So before going to sleep ensure that all stations
> got deauthenticated.

Also your text and subject and code is really confusing -- I thought you
wanted to deauthenticate stations that are associated to mac80211 acting
as an AP.

johannes

Re: [RFC] mac80211: deauthenticate STAs before suspend

[Rajkumar Manoharan]

On Mon, Jan 31, 2011 at 04:57:41PM +0530, Johannes Berg wrote:
> On Mon, 2011-01-31 at 15:36 +0530, Rajkumar Manoharan wrote:
> > The STA is not deauthenticated before the system goes to sleep,
> > when the vif was configured by iw/iwconfig. This holds the AP
> > resources unnecessarily, till the STA entry got expired in AP.
> 
> Is that a big problem? We actually had this intentionally, and we go to
> PS before going to sleep, so if the sleep is very short then we will not
> be disconnected. Also, it's much easier this way to implement WoWLAN.
I looked it as a problem ;) And if we use NM/supplicant, obviously the STA
got disconnected. So just to align with that I sent the RFC.

> > So before going to sleep ensure that all stations
> > got deauthenticated.
> 
> Also your text and subject and code is really confusing -- I thought you
> wanted to deauthenticate stations that are associated to mac80211 acting
> as an AP.
I meant that the stations associated in all vifs.

--
Rajkumar

Re: [RFC] mac80211: deauthenticate STAs before suspend

[Jouni Malinen]

On Mon, Jan 31, 2011 at 05:08:35PM +0530, Rajkumar Manoharan wrote:
> On Mon, Jan 31, 2011 at 04:57:41PM +0530, Johannes Berg wrote:
> > Is that a big problem? We actually had this intentionally, and we go to
> > PS before going to sleep, so if the sleep is very short then we will not
> > be disconnected. Also, it's much easier this way to implement WoWLAN.
> I looked it as a problem ;) And if we use NM/supplicant, obviously the STA
> got disconnected. So just to align with that I sent the RFC.

While this may behave differently if NM is running (wpa_supplicant
should not be being the disconnection on suspend), the comment about
Wake-on-WLAN is something that should be considered here. I would expect
there to be some interest in being able to support it and forcing a
disconnection on suspend would make that impossible. In addition,
extended PS mode from 802.11v will make it even more desirable to be
able to sleep for extended periods of time without dropping the
association for some use cases. If we force a disconnection here, we
would need to have a flag for disabling that if there is any chance of
the association being of use for something else after the resume or if
Wake-on-WLAN is enabled.
 
-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [RFC] mac80211: deauthenticate STAs before suspend

[Johannes Berg]

On Mon, 2011-01-31 at 13:47 +0200, Jouni Malinen wrote:
> On Mon, Jan 31, 2011 at 05:08:35PM +0530, Rajkumar Manoharan wrote:
> > On Mon, Jan 31, 2011 at 04:57:41PM +0530, Johannes Berg wrote:
> > > Is that a big problem? We actually had this intentionally, and we go to
> > > PS before going to sleep, so if the sleep is very short then we will not
> > > be disconnected. Also, it's much easier this way to implement WoWLAN.
> > I looked it as a problem ;) And if we use NM/supplicant, obviously the STA
> > got disconnected. So just to align with that I sent the RFC.
> 
> While this may behave differently if NM is running (wpa_supplicant
> should not be being the disconnection on suspend), the comment about
> Wake-on-WLAN is something that should be considered here. I would expect
> there to be some interest in being able to support it and forcing a
> disconnection on suspend would make that impossible. In addition,
> extended PS mode from 802.11v will make it even more desirable to be
> able to sleep for extended periods of time without dropping the
> association for some use cases. If we force a disconnection here, we
> would need to have a flag for disabling that if there is any chance of
> the association being of use for something else after the resume or if
> Wake-on-WLAN is enabled.

I just don't see why we need the additional complexity of disabling it
and then re-enabling it conditionally when we want WoWL or 11v extended
PS?

IOW, I'd rather leave it as is. It seems there isn't really a problem,
but rather a perceived inconsistency that is being "fixed"?

johannes

[RFC] mac80211: deauthenticate STAs before suspend

[Sujith]

Rajkumar Manoharan wrote:
>  #ifdef CONFIG_PM
> +static int ieee80211_pre_suspend(struct wiphy *wiphy)
> +{
> +	struct ieee80211_local *local = wiphy_priv(wiphy);
> +	struct ieee80211_sub_if_data *sdata = NULL;
> +	struct ieee80211_if_managed *ifmgd = NULL;
> +	struct cfg80211_deauth_request req;
> +
> +	list_for_each_entry(sdata, &local->interfaces, list) {
> +		ifmgd = &sdata->u.mgd;

Shouldn't the interface type be checked here ?

> +	.pre_suspend = ieee80211_pre_suspend,
>  	.suspend = ieee80211_suspend,
>  	.resume = ieee80211_resume,
>  	.scan = ieee80211_scan,
> diff --git a/net/wireless/sysfs.c b/net/wireless/sysfs.c
> index 4294fa2..944d589 100644
> --- a/net/wireless/sysfs.c
> +++ b/net/wireless/sysfs.c
> @@ -91,6 +91,12 @@ static int wiphy_suspend(struct device *dev, pm_message_t state)
>  
>  	rdev->suspend_at = get_seconds();
>  
> +	if (rdev->ops->pre_suspend) {
> +		rtnl_lock();
> +		ret = rdev->ops->pre_suspend(&rdev->wiphy);
> +		rtnl_unlock();
> +	}
> +

Why a 'pre-suspend' ? :)
Can't this be done in __ieee80211_suspend() ?

Sujith