2012-12-14 11:59:09

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 0/3] mac80211: suspend fixes/cleanups

Some things I found while testing suspend ...

johannes



2012-12-14 11:59:10

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 2/3] mac80211: clean up association better in suspend

From: Johannes Berg <[email protected]>

When suspending, bss_info_changed() is called to
disable beacons, but managed mode interfaces are
simply removed (bss_info_changed() is called with
"no change" only). This can lead to problems.

To fix this and copy the BSS configuration, clear
it during suspend and restore it on resume.

Change-Id: Ie1b7f6343fe59dfbff886b9720e4c15686d7fda7
Signed-off-by: Johannes Berg <[email protected]>
---
net/mac80211/ieee80211_i.h | 5 +++++
net/mac80211/pm.c | 13 ++++++++++---
net/mac80211/util.c | 5 +++++
3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 8baaeda..0360b74 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -785,6 +785,11 @@ struct ieee80211_sub_if_data {
struct dentry *default_mgmt_key;
} 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/pm.c b/net/mac80211/pm.c
index f321783..f3a9e5f 100644
--- a/net/mac80211/pm.c
+++ b/net/mac80211/pm.c
@@ -121,6 +121,8 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)

/* remove all interfaces */
list_for_each_entry(sdata, &local->interfaces, list) {
+ u32 changed = BSS_CHANGED_BEACON_ENABLED;
+
if (!ieee80211_sdata_running(sdata))
continue;

@@ -129,14 +131,19 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
case NL80211_IFTYPE_MONITOR:
/* skip these */
continue;
+ case NL80211_IFTYPE_STATION:
+ changed = BSS_CHANGED_ASSOC;
+ /* fall through */
default:
ieee80211_quiesce(sdata);
break;
}

- /* disable beaconing */
- ieee80211_bss_info_change_notify(sdata,
- BSS_CHANGED_BEACON_ENABLED);
+ sdata->suspend_bss_conf = sdata->vif.bss_conf;
+ memset(&sdata->vif.bss_conf, 0, sizeof(sdata->vif.bss_conf));
+
+ /* 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))
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 6ba0c66..c170b9d 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1533,6 +1533,11 @@ int ieee80211_reconfig(struct ieee80211_local *local)
BSS_CHANGED_IDLE |
BSS_CHANGED_TXPOWER;

+#ifdef CONFIG_PM
+ if (local->resuming)
+ sdata->vif.bss_conf = sdata->suspend_bss_conf;
+#endif
+
switch (sdata->vif.type) {
case NL80211_IFTYPE_STATION:
changed |= BSS_CHANGED_ASSOC |
--
1.8.0


2012-12-14 11:59:10

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 1/3] mac80211: clean up ieee80211_quiesce

From: Johannes Berg <[email protected]>

It's a bit odd that there's a return value that only
depends on the iftype, move that logic out of the
function into the only caller that needs it.

Also, since the quiescing could stop timers that
trigger the sdata work, move the sdata work cancel
into the function and after the actual quiesce.

Finally, there's no need to call it on interfaces
that are down, so don't.

Change-Id: I1632d46d21ba3558ea713d035184f1939905f2f1
Signed-off-by: Johannes Berg <[email protected]>
---
net/mac80211/pm.c | 36 +++++++++++++++++++-----------------
1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/net/mac80211/pm.c b/net/mac80211/pm.c
index 79a48f3..f321783 100644
--- a/net/mac80211/pm.c
+++ b/net/mac80211/pm.c
@@ -7,25 +7,23 @@
#include "led.h"

/* return value indicates whether the driver should be further notified */
-static bool ieee80211_quiesce(struct ieee80211_sub_if_data *sdata)
+static void ieee80211_quiesce(struct ieee80211_sub_if_data *sdata)
{
switch (sdata->vif.type) {
case NL80211_IFTYPE_STATION:
ieee80211_sta_quiesce(sdata);
- return true;
+ break;
case NL80211_IFTYPE_ADHOC:
ieee80211_ibss_quiesce(sdata);
- return true;
+ break;
case NL80211_IFTYPE_MESH_POINT:
ieee80211_mesh_quiesce(sdata);
- return true;
- case NL80211_IFTYPE_AP_VLAN:
- case NL80211_IFTYPE_MONITOR:
- /* don't tell driver about this */
- return false;
+ break;
default:
- return true;
+ break;
}
+
+ cancel_work_sync(&sdata->work);
}

int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
@@ -94,10 +92,9 @@ 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) {
- cancel_work_sync(&sdata->work);
- ieee80211_quiesce(sdata);
- }
+ list_for_each_entry(sdata, &local->interfaces, list)
+ if (ieee80211_sdata_running(sdata))
+ ieee80211_quiesce(sdata);
goto suspend;
}
}
@@ -124,13 +121,18 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)

/* remove all interfaces */
list_for_each_entry(sdata, &local->interfaces, list) {
- cancel_work_sync(&sdata->work);
-
- if (!ieee80211_quiesce(sdata))
+ if (!ieee80211_sdata_running(sdata))
continue;

- if (!ieee80211_sdata_running(sdata))
+ switch (sdata->vif.type) {
+ case NL80211_IFTYPE_AP_VLAN:
+ case NL80211_IFTYPE_MONITOR:
+ /* skip these */
continue;
+ default:
+ ieee80211_quiesce(sdata);
+ break;
+ }

/* disable beaconing */
ieee80211_bss_info_change_notify(sdata,
--
1.8.0


2012-12-14 11:59:10

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 3/3] mac80211: fix channel context iteration

From: Johannes Berg <[email protected]>

During suspend/resume channel contexts might be
iterated even if they haven't been re-added to
the driver, keep track of this and skip them in
iteration. Also use the new status for sanity
checks.

Change-Id: Ibac29191dcc24578d56565c36a543d7a9733be10
Signed-off-by: Johannes Berg <[email protected]>
---
net/mac80211/chan.c | 3 ++-
net/mac80211/driver-ops.h | 15 ++++++++++++---
net/mac80211/ieee80211_i.h | 1 +
3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index 80e5552..1bfe0a8 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -381,7 +381,8 @@ void ieee80211_iter_chan_contexts_atomic(

rcu_read_lock();
list_for_each_entry_rcu(ctx, &local->chanctx_list, list)
- iter(hw, &ctx->conf, iter_data);
+ if (ctx->driver_present)
+ iter(hw, &ctx->conf, iter_data);
rcu_read_unlock();
}
EXPORT_SYMBOL_GPL(ieee80211_iter_chan_contexts_atomic);
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index c6560cc..70dcc42 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -913,6 +913,8 @@ static inline int drv_add_chanctx(struct ieee80211_local *local,
if (local->ops->add_chanctx)
ret = local->ops->add_chanctx(&local->hw, &ctx->conf);
trace_drv_return_int(local, ret);
+ if (!ret)
+ ctx->driver_present = true;

return ret;
}
@@ -924,6 +926,7 @@ static inline void drv_remove_chanctx(struct ieee80211_local *local,
if (local->ops->remove_chanctx)
local->ops->remove_chanctx(&local->hw, &ctx->conf);
trace_drv_return_void(local);
+ ctx->driver_present = false;
}

static inline void drv_change_chanctx(struct ieee80211_local *local,
@@ -931,8 +934,10 @@ static inline void drv_change_chanctx(struct ieee80211_local *local,
u32 changed)
{
trace_drv_change_chanctx(local, ctx, changed);
- if (local->ops->change_chanctx)
+ if (local->ops->change_chanctx) {
+ WARN_ON_ONCE(!ctx->driver_present);
local->ops->change_chanctx(&local->hw, &ctx->conf, changed);
+ }
trace_drv_return_void(local);
}

@@ -945,10 +950,12 @@ static inline int drv_assign_vif_chanctx(struct ieee80211_local *local,
check_sdata_in_driver(sdata);

trace_drv_assign_vif_chanctx(local, sdata, ctx);
- if (local->ops->assign_vif_chanctx)
+ if (local->ops->assign_vif_chanctx) {
+ WARN_ON_ONCE(!ctx->driver_present);
ret = local->ops->assign_vif_chanctx(&local->hw,
&sdata->vif,
&ctx->conf);
+ }
trace_drv_return_int(local, ret);

return ret;
@@ -961,10 +968,12 @@ static inline void drv_unassign_vif_chanctx(struct ieee80211_local *local,
check_sdata_in_driver(sdata);

trace_drv_unassign_vif_chanctx(local, sdata, ctx);
- if (local->ops->unassign_vif_chanctx)
+ if (local->ops->unassign_vif_chanctx) {
+ WARN_ON_ONCE(!ctx->driver_present);
local->ops->unassign_vif_chanctx(&local->hw,
&sdata->vif,
&ctx->conf);
+ }
trace_drv_return_void(local);
}

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 0360b74..2bb0c98 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -687,6 +687,7 @@ struct ieee80211_chanctx {

enum ieee80211_chanctx_mode mode;
int refcount;
+ bool driver_present;

struct ieee80211_chanctx_conf conf;
};
--
1.8.0


2012-12-19 10:52:10

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 3/3] mac80211: fix channel context iteration

On Sun, 2012-12-16 at 11:02 +0200, Eliad Peller wrote:
> On Fri, Dec 14, 2012 at 1:59 PM, Johannes Berg
> <[email protected]> wrote:
> > From: Johannes Berg <[email protected]>
> >
> > During suspend/resume channel contexts might be
> > iterated even if they haven't been re-added to
> > the driver, keep track of this and skip them in
> > iteration. Also use the new status for sanity
> > checks.
> >
> > Change-Id: Ibac29191dcc24578d56565c36a543d7a9733be10
> > Signed-off-by: Johannes Berg <[email protected]>
> > ---
> > net/mac80211/chan.c | 3 ++-
> > net/mac80211/driver-ops.h | 15 ++++++++++++---
> > net/mac80211/ieee80211_i.h | 1 +
> > 3 files changed, 15 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
> > index 80e5552..1bfe0a8 100644
> > --- a/net/mac80211/chan.c
> > +++ b/net/mac80211/chan.c
> > @@ -381,7 +381,8 @@ void ieee80211_iter_chan_contexts_atomic(
> >
> > rcu_read_lock();
> > list_for_each_entry_rcu(ctx, &local->chanctx_list, list)
> > - iter(hw, &ctx->conf, iter_data);
> > + if (ctx->driver_present)
> > + iter(hw, &ctx->conf, iter_data);
> > rcu_read_unlock();
> > }
> this will behave differently on hw restart (as ctx->driver_present
> will remain true).
> at least worth documenting :)

Yes, good point.

johannes


From: Johannes Berg <[email protected]>
Date: Thu, 13 Dec 2012 17:42:30 +0100
Subject: [PATCH] mac80211: fix channel context iteration

During suspend/resume channel contexts might be
iterated even if they haven't been re-added to
the driver, keep track of this and skip them in
iteration. Also use the new status for sanity
checks.

Also clarify the fact that during HW restart all
contexts are iterated over (thanks Eliad.)

Signed-off-by: Johannes Berg <[email protected]>
---
include/net/mac80211.h | 5 +++++
net/mac80211/chan.c | 3 ++-
net/mac80211/driver-ops.h | 15 ++++++++++++---
net/mac80211/ieee80211_i.h | 1 +
4 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index ee50c5e..0978b0f 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -3754,6 +3754,11 @@ void ieee80211_iter_keys(struct ieee80211_hw *hw,
* The iterator will not find a context that's being added (during
* the driver callback to add it) but will find it while it's being
* removed.
+ *
+ * Note that during hardware restart, all contexts that existed
+ * before the restart are considered already present so will be
+ * found while iterating, whether they've been re-added already
+ * or not.
*/
void ieee80211_iter_chan_contexts_atomic(
struct ieee80211_hw *hw,
diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index 80e5552..1bfe0a8 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -381,7 +381,8 @@ void ieee80211_iter_chan_contexts_atomic(

rcu_read_lock();
list_for_each_entry_rcu(ctx, &local->chanctx_list, list)
- iter(hw, &ctx->conf, iter_data);
+ if (ctx->driver_present)
+ iter(hw, &ctx->conf, iter_data);
rcu_read_unlock();
}
EXPORT_SYMBOL_GPL(ieee80211_iter_chan_contexts_atomic);
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index c6560cc..70dcc42 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -913,6 +913,8 @@ static inline int drv_add_chanctx(struct ieee80211_local *local,
if (local->ops->add_chanctx)
ret = local->ops->add_chanctx(&local->hw, &ctx->conf);
trace_drv_return_int(local, ret);
+ if (!ret)
+ ctx->driver_present = true;

return ret;
}
@@ -924,6 +926,7 @@ static inline void drv_remove_chanctx(struct ieee80211_local *local,
if (local->ops->remove_chanctx)
local->ops->remove_chanctx(&local->hw, &ctx->conf);
trace_drv_return_void(local);
+ ctx->driver_present = false;
}

static inline void drv_change_chanctx(struct ieee80211_local *local,
@@ -931,8 +934,10 @@ static inline void drv_change_chanctx(struct ieee80211_local *local,
u32 changed)
{
trace_drv_change_chanctx(local, ctx, changed);
- if (local->ops->change_chanctx)
+ if (local->ops->change_chanctx) {
+ WARN_ON_ONCE(!ctx->driver_present);
local->ops->change_chanctx(&local->hw, &ctx->conf, changed);
+ }
trace_drv_return_void(local);
}

@@ -945,10 +950,12 @@ static inline int drv_assign_vif_chanctx(struct ieee80211_local *local,
check_sdata_in_driver(sdata);

trace_drv_assign_vif_chanctx(local, sdata, ctx);
- if (local->ops->assign_vif_chanctx)
+ if (local->ops->assign_vif_chanctx) {
+ WARN_ON_ONCE(!ctx->driver_present);
ret = local->ops->assign_vif_chanctx(&local->hw,
&sdata->vif,
&ctx->conf);
+ }
trace_drv_return_int(local, ret);

return ret;
@@ -961,10 +968,12 @@ static inline void drv_unassign_vif_chanctx(struct ieee80211_local *local,
check_sdata_in_driver(sdata);

trace_drv_unassign_vif_chanctx(local, sdata, ctx);
- if (local->ops->unassign_vif_chanctx)
+ if (local->ops->unassign_vif_chanctx) {
+ WARN_ON_ONCE(!ctx->driver_present);
local->ops->unassign_vif_chanctx(&local->hw,
&sdata->vif,
&ctx->conf);
+ }
trace_drv_return_void(local);
}

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 0360b74..2bb0c98 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -687,6 +687,7 @@ struct ieee80211_chanctx {

enum ieee80211_chanctx_mode mode;
int refcount;
+ bool driver_present;

struct ieee80211_chanctx_conf conf;
};
--
1.8.0




2012-12-19 10:57:01

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/3] mac80211: clean up association better in suspend

On Sun, 2012-12-16 at 10:55 +0200, Eliad Peller wrote:

> > + case NL80211_IFTYPE_STATION:
> > + changed = BSS_CHANGED_ASSOC;
>
> maybe set it only if associated?

good point

> also, is there really a point in notifying BSS_CHANGED_ASSOC without
> notifying CHANGED_BSSID (and maybe CHANGED_IDLE)?

Hmm, I guess if associated the BSSID will change, and IDLE will change.

I'll add those flags. I guess it depends on how the driver handles it.

I have a feeling we should have a bit smarter managed mode APIs, like I
recently added for AP mode (start_ap/stop_ap), the BSS info change
doesn't make sense to randomly change the BSSID while associated for
example, but it's redundant in that the driver might look at the BSSID
change or the associated change and do something, or treat them
separately (which could lead to confusion & thinking that they can
change separately...)

> > - /* disable beaconing */
> > - ieee80211_bss_info_change_notify(sdata,
> > - BSS_CHANGED_BEACON_ENABLED);
> > + sdata->suspend_bss_conf = sdata->vif.bss_conf;
> > + memset(&sdata->vif.bss_conf, 0, sizeof(sdata->vif.bss_conf));
>
> it probably doesn't matter much (as you don't notify CHANGED_IDLE),
> but setting vif.idle=0 is probably not your intention here.

Yeah I'll set it to true.

johannes

>From 5fe27effbc755f368c57981fe50143d649541f5b Mon Sep 17 00:00:00 2001
From: Johannes Berg <[email protected]>
Date: Thu, 13 Dec 2012 17:16:45 +0100
Subject: [PATCH] mac80211: clean up association better in suspend

When suspending, bss_info_changed() is called to
disable beacons, but managed mode interfaces are
simply removed (bss_info_changed() is called with
"no change" only). This can lead to problems.

To fix this and copy the BSS configuration, clear
it during suspend and restore it on resume.

Signed-off-by: Johannes Berg <[email protected]>
---
net/mac80211/ieee80211_i.h | 5 +++++
net/mac80211/pm.c | 19 ++++++++++++++++---
net/mac80211/util.c | 5 +++++
3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 8baaeda..0360b74 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -785,6 +785,11 @@ struct ieee80211_sub_if_data {
struct dentry *default_mgmt_key;
} 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/pm.c b/net/mac80211/pm.c
index f321783..712c17f 100644
--- a/net/mac80211/pm.c
+++ b/net/mac80211/pm.c
@@ -121,6 +121,8 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)

/* remove all interfaces */
list_for_each_entry(sdata, &local->interfaces, list) {
+ u32 changed = BSS_CHANGED_BEACON_ENABLED;
+
if (!ieee80211_sdata_running(sdata))
continue;

@@ -129,14 +131,25 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
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;
+ else
+ changed = 0;
+ /* fall through */
default:
ieee80211_quiesce(sdata);
break;
}

- /* disable beaconing */
- ieee80211_bss_info_change_notify(sdata,
- BSS_CHANGED_BEACON_ENABLED);
+ 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;
+
+ /* 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))
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 6ba0c66..c170b9d 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1533,6 +1533,11 @@ int ieee80211_reconfig(struct ieee80211_local *local)
BSS_CHANGED_IDLE |
BSS_CHANGED_TXPOWER;

+#ifdef CONFIG_PM
+ if (local->resuming)
+ sdata->vif.bss_conf = sdata->suspend_bss_conf;
+#endif
+
switch (sdata->vif.type) {
case NL80211_IFTYPE_STATION:
changed |= BSS_CHANGED_ASSOC |
--
1.8.0




2012-12-16 09:02:04

by Eliad Peller

[permalink] [raw]
Subject: Re: [PATCH 3/3] mac80211: fix channel context iteration

On Fri, Dec 14, 2012 at 1:59 PM, Johannes Berg
<[email protected]> wrote:
> From: Johannes Berg <[email protected]>
>
> During suspend/resume channel contexts might be
> iterated even if they haven't been re-added to
> the driver, keep track of this and skip them in
> iteration. Also use the new status for sanity
> checks.
>
> Change-Id: Ibac29191dcc24578d56565c36a543d7a9733be10
> Signed-off-by: Johannes Berg <[email protected]>
> ---
> net/mac80211/chan.c | 3 ++-
> net/mac80211/driver-ops.h | 15 ++++++++++++---
> net/mac80211/ieee80211_i.h | 1 +
> 3 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
> index 80e5552..1bfe0a8 100644
> --- a/net/mac80211/chan.c
> +++ b/net/mac80211/chan.c
> @@ -381,7 +381,8 @@ void ieee80211_iter_chan_contexts_atomic(
>
> rcu_read_lock();
> list_for_each_entry_rcu(ctx, &local->chanctx_list, list)
> - iter(hw, &ctx->conf, iter_data);
> + if (ctx->driver_present)
> + iter(hw, &ctx->conf, iter_data);
> rcu_read_unlock();
> }
this will behave differently on hw restart (as ctx->driver_present
will remain true).
at least worth documenting :)

Eliad.

2012-12-16 08:55:52

by Eliad Peller

[permalink] [raw]
Subject: Re: [PATCH 2/3] mac80211: clean up association better in suspend

On Fri, Dec 14, 2012 at 1:59 PM, Johannes Berg
<[email protected]> wrote:
> From: Johannes Berg <[email protected]>
>
> When suspending, bss_info_changed() is called to
> disable beacons, but managed mode interfaces are
> simply removed (bss_info_changed() is called with
> "no change" only). This can lead to problems.
>
> To fix this and copy the BSS configuration, clear
> it during suspend and restore it on resume.
>
> Change-Id: Ie1b7f6343fe59dfbff886b9720e4c15686d7fda7
> Signed-off-by: Johannes Berg <[email protected]>
> ---

> @@ -121,6 +121,8 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
> @@ -129,14 +131,19 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
> case NL80211_IFTYPE_MONITOR:
> /* skip these */
> continue;
> + case NL80211_IFTYPE_STATION:
> + changed = BSS_CHANGED_ASSOC;

maybe set it only if associated?
also, is there really a point in notifying BSS_CHANGED_ASSOC without
notifying CHANGED_BSSID (and maybe CHANGED_IDLE)?

> - /* disable beaconing */
> - ieee80211_bss_info_change_notify(sdata,
> - BSS_CHANGED_BEACON_ENABLED);
> + sdata->suspend_bss_conf = sdata->vif.bss_conf;
> + memset(&sdata->vif.bss_conf, 0, sizeof(sdata->vif.bss_conf));

it probably doesn't matter much (as you don't notify CHANGED_IDLE),
but setting vif.idle=0 is probably not your intention here.

2012-12-20 11:32:30

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 0/3] mac80211: suspend fixes/cleanups

Applied all.

johannes