[PATCH v3 1/1] mac80211: tell driver when dtim change detected

[PATCH v3 1/1] mac80211: tell driver when dtim change detected

[wey-yi.w.guy]

From: Wey-Yi Guy <wey-yi.w.guy@intel.com>

In current implementation, mac80211 send dtim_period update to driver
during association, but if no NetworkManager or similar application
perform scan operation, plus tim_ie is not part of probe response; mac80211 will
not get beacon with dtim information later, then  mac80211 will not pass the
information to driver for update.

Call ieee80211_hw_config() with IEEE80211_CONF_CHANGE_PS flag set to
allow driver make correct dtim adjustment if dtim_period change
detected. Also perform recalc_ps operation if needed.

Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
---
v2: move the function to ieee80211_rx_bss_info() to make sure only call
    when needed
v3: do not check for STATION and call recal_ps to perform hw_config
---
 net/mac80211/mlme.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 1e1d16c..46df5bf 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1157,6 +1157,21 @@ static bool ieee80211_assoc_success(struct ieee80211_work *wk,
 }
 
 
+static void ieee80211_update_dtim(struct ieee80211_sub_if_data *sdata,
+				  struct ieee80211_local *local,
+				  struct ieee80211_bss *bss)
+{
+	if (!(local->hw.flags & IEEE80211_HW_SUPPORTS_PS))
+		return;
+
+	if (sdata->vif.bss_conf.dtim_period != bss->dtim_period) {
+		sdata->vif.bss_conf.dtim_period = bss->dtim_period;
+		mutex_lock(&local->iflist_mtx);
+		ieee80211_recalc_ps(local, -1);
+		mutex_unlock(&local->iflist_mtx);
+	}
+}
+
 static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
 				  struct ieee80211_mgmt *mgmt,
 				  size_t len,
@@ -1181,8 +1196,10 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
 
 	bss = ieee80211_bss_info_update(local, rx_status, mgmt, len, elems,
 					channel, beacon);
-	if (bss)
+	if (bss) {
 		ieee80211_rx_bss_put(local, bss);
+		ieee80211_update_dtim(sdata, local, bss);
+	}
 
 	if (!sdata->u.mgd.associated)
 		return;
-- 
1.5.6.3

Re: [PATCH v3 1/1] mac80211: tell driver when dtim change detected

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 2163 bytes --]

[adding Jouni]

On Thu, 2010-01-21 at 13:39 -0800, wey-yi.w.guy@intel.com wrote:
> From: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> 
> In current implementation, mac80211 send dtim_period update to driver
> during association, but if no NetworkManager or similar application
> perform scan operation, plus tim_ie is not part of probe response; mac80211 will
> not get beacon with dtim information later, then  mac80211 will not pass the
> information to driver for update.
> 
> Call ieee80211_hw_config() with IEEE80211_CONF_CHANGE_PS flag set to
> allow driver make correct dtim adjustment if dtim_period change
> detected. Also perform recalc_ps operation if needed.

This seems fine.

However, I think it's indicative of a bigger problem. I gave it some
thought, and came to the conclusion that it previously didn't happen
because we always won the race. Let me explain.

Previously, we would switch the channel completely to the new operating
channel before even probing the AP. That way, we would virtually always
receive a beacon from the new AP between the time we started the
association process (probe,auth,assoc) and configuring the driver.

Now with the new changes that use the off-channel work, we may return to
the old "operating" channel, which may be no particular channel, between
all these steps. Thus, if there's no beacon between any of probe
request/response, auth "request"/"response", assoc request/response, we
never get one, and this situation happens.

I see two solutions, apart from this special-case patch fixing 

First, we could go back to the original behaviour if we have just one
virtual interface. But that still leaves us with the race, we might do
all three frame exchanges within a beacon interval and still miss the
beacon, we just tend to not do that and get a beacon.

The other solution I see is that we add a new step before or after the
direct probe step, which would just be "wait for a beacon". This would
ensure we have both probe and beacon information always ready. It would
also ensure we have both probe and beacon info for our new userspace
reporting of that.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v3 1/1] mac80211: tell driver when dtim change detected

[Luis R. Rodriguez]

On Fri, Jan 22, 2010 at 11:03 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> [adding Jouni]
>
> On Thu, 2010-01-21 at 13:39 -0800, wey-yi.w.guy@intel.com wrote:
>> From: Wey-Yi Guy <wey-yi.w.guy@intel.com>
>>
>> In current implementation, mac80211 send dtim_period update to driver
>> during association, but if no NetworkManager or similar application
>> perform scan operation, plus tim_ie is not part of probe response; mac80211 will
>> not get beacon with dtim information later, then  mac80211 will not pass the
>> information to driver for update.
>>
>> Call ieee80211_hw_config() with IEEE80211_CONF_CHANGE_PS flag set to
>> allow driver make correct dtim adjustment if dtim_period change
>> detected. Also perform recalc_ps operation if needed.
>
> This seems fine.
>
> However, I think it's indicative of a bigger problem. I gave it some
> thought, and came to the conclusion that it previously didn't happen
> because we always won the race. Let me explain.
>
> Previously, we would switch the channel completely to the new operating
> channel before even probing the AP. That way, we would virtually always
> receive a beacon from the new AP between the time we started the
> association process (probe,auth,assoc) and configuring the driver.
>
> Now with the new changes that use the off-channel work, we may return to
> the old "operating" channel, which may be no particular channel, between
> all these steps. Thus, if there's no beacon between any of probe
> request/response, auth "request"/"response", assoc request/response, we
> never get one, and this situation happens.
>
> I see two solutions, apart from this special-case patch fixing
>
> First, we could go back to the original behaviour if we have just one
> virtual interface. But that still leaves us with the race, we might do
> all three frame exchanges within a beacon interval and still miss the
> beacon, we just tend to not do that and get a beacon.

Curious, what symptoms were seen when the dtim was not propagated
prior to association, did the STA just not wake up for the right dtim
interval when in PS mode? Wouldn't we get the dtim interval on
eventual beacons later or do we disregard all that information after
associated? If so what if the AP changes the dtim interval?

I take it this can easily be reproduced with a long beacon interval?

> The other solution I see is that we add a new step before or after the
> direct probe step, which would just be "wait for a beacon". This would
> ensure we have both probe and beacon information always ready. It would
> also ensure we have both probe and beacon info for our new userspace
> reporting of that.

This seems cleaner.

  Luis

Re: [PATCH v3 1/1] mac80211: tell driver when dtim change detected

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 2967 bytes --]

On Fri, 2010-01-22 at 11:20 -0800, Luis R. Rodriguez wrote:

> > Previously, we would switch the channel completely to the new operating
> > channel before even probing the AP. That way, we would virtually always
> > receive a beacon from the new AP between the time we started the
> > association process (probe,auth,assoc) and configuring the driver.
> >
> > Now with the new changes that use the off-channel work, we may return to
> > the old "operating" channel, which may be no particular channel, between
> > all these steps. Thus, if there's no beacon between any of probe
> > request/response, auth "request"/"response", assoc request/response, we
> > never get one, and this situation happens.
> >
> > I see two solutions, apart from this special-case patch fixing
> >
> > First, we could go back to the original behaviour if we have just one
> > virtual interface. But that still leaves us with the race, we might do
> > all three frame exchanges within a beacon interval and still miss the
> > beacon, we just tend to not do that and get a beacon.
> 
> Curious, what symptoms were seen when the dtim was not propagated
> prior to association, did the STA just not wake up for the right dtim
> interval when in PS mode? 

Yes, I don't really know exactly what happens, but if you look at
Wey-Yi's patch you'll see most of the problem was that we calculated the
maximum powersaving interval wrong.

> Wouldn't we get the dtim interval on
> eventual beacons later or do we disregard all that information after
> associated?

As you can see from the patch, we currently disregard any "update" in
that information -- this patch changes it.

> If so what if the AP changes the dtim interval?

I believe the AP is not allowed to do that without turning off and back
on completely.

> I take it this can easily be reproduced with a long beacon interval?

Easier, yes. The easiest way to reproduce it is to associate using just
"iw", no NM or wpa_supplicant -- make sure the scan list is empty before
you associate -- and with a long beacon interval. We then scan, but that
will only give us a probe response with high probability, and then we do
all the switching as I explained and we don't ever get a beacon. If you
use NM it may scan more often and increase the probability of us having
a beacon at some point in time.

> > The other solution I see is that we add a new step before or after the
> > direct probe step, which would just be "wait for a beacon". This would
> > ensure we have both probe and beacon information always ready. It would
> > also ensure we have both probe and beacon info for our new userspace
> > reporting of that.
> 
> This seems cleaner.

I agree. And since we already keep track of this since 34a6eddb, we
should be able to fairly easily determine whether we still need probe
response or beacon, and only do either the direct probe or wait for
beacon step.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v3 1/1] mac80211: tell driver when dtim change detected

[Luis R. Rodriguez]

On Fri, Jan 22, 2010 at 11:46 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Fri, 2010-01-22 at 11:20 -0800, Luis R. Rodriguez wrote:
>
>> > Previously, we would switch the channel completely to the new operating
>> > channel before even probing the AP. That way, we would virtually always
>> > receive a beacon from the new AP between the time we started the
>> > association process (probe,auth,assoc) and configuring the driver.
>> >
>> > Now with the new changes that use the off-channel work, we may return to
>> > the old "operating" channel, which may be no particular channel, between
>> > all these steps. Thus, if there's no beacon between any of probe
>> > request/response, auth "request"/"response", assoc request/response, we
>> > never get one, and this situation happens.
>> >
>> > I see two solutions, apart from this special-case patch fixing
>> >
>> > First, we could go back to the original behaviour if we have just one
>> > virtual interface. But that still leaves us with the race, we might do
>> > all three frame exchanges within a beacon interval and still miss the
>> > beacon, we just tend to not do that and get a beacon.
>>
>> Curious, what symptoms were seen when the dtim was not propagated
>> prior to association, did the STA just not wake up for the right dtim
>> interval when in PS mode?
>
> Yes, I don't really know exactly what happens,

OK -- Wey, can you elaborate a little as to how you found this and
what you were seeing? Does the device not go into PS mode at all?

>  but if you look at
> Wey-Yi's patch you'll see most of the problem was that we calculated the
> maximum powersaving interval wrong.

I see..

>> Wouldn't we get the dtim interval on
>> eventual beacons later or do we disregard all that information after
>> associated?
>
> As you can see from the patch, we currently disregard any "update" in
> that information -- this patch changes it.

Got it.

>> If so what if the AP changes the dtim interval?
>
> I believe the AP is not allowed to do that without turning off and back
> on completely.

OK. Still curious, if in practice would funky APs ever tune DTIM
dynamically and not kick you off, and if they do is it fair to ignore
that and blame the AP?

>> I take it this can easily be reproduced with a long beacon interval?
>
> Easier, yes. The easiest way to reproduce it is to associate using just
> "iw", no NM or wpa_supplicant -- make sure the scan list is empty before
> you associate -- and with a long beacon interval. We then scan, but that
> will only give us a probe response with high probability, and then we do
> all the switching as I explained and we don't ever get a beacon. If you
> use NM it may scan more often and increase the probability of us having
> a beacon at some point in time.

OK thanks.

>> > The other solution I see is that we add a new step before or after the
>> > direct probe step, which would just be "wait for a beacon". This would
>> > ensure we have both probe and beacon information always ready. It would
>> > also ensure we have both probe and beacon info for our new userspace
>> > reporting of that.
>>
>> This seems cleaner.
>
> I agree. And since we already keep track of this since 34a6eddb, we
> should be able to fairly easily determine whether we still need probe
> response or beacon, and only do either the direct probe or wait for
> beacon step.

  Luis

Re: [PATCH v3 1/1] mac80211: tell driver when dtim change detected

[Luis R. Rodriguez]

On Fri, Jan 22, 2010 at 3:44 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
> On Fri, Jan 22, 2010 at 11:46 AM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
>> On Fri, 2010-01-22 at 11:20 -0800, Luis R. Rodriguez wrote:
>>
>>> > Previously, we would switch the channel completely to the new operating
>>> > channel before even probing the AP. That way, we would virtually always
>>> > receive a beacon from the new AP between the time we started the
>>> > association process (probe,auth,assoc) and configuring the driver.
>>> >
>>> > Now with the new changes that use the off-channel work, we may return to
>>> > the old "operating" channel, which may be no particular channel, between
>>> > all these steps. Thus, if there's no beacon between any of probe
>>> > request/response, auth "request"/"response", assoc request/response, we
>>> > never get one, and this situation happens.
>>> >
>>> > I see two solutions, apart from this special-case patch fixing
>>> >
>>> > First, we could go back to the original behaviour if we have just one
>>> > virtual interface. But that still leaves us with the race, we might do
>>> > all three frame exchanges within a beacon interval and still miss the
>>> > beacon, we just tend to not do that and get a beacon.
>>>
>>> Curious, what symptoms were seen when the dtim was not propagated
>>> prior to association, did the STA just not wake up for the right dtim
>>> interval when in PS mode?
>>
>> Yes, I don't really know exactly what happens,
>
> OK -- Wey, can you elaborate a little as to how you found this and
> what you were seeing? Does the device not go into PS mode at all?
>

BTW I ask more details because I'm trying to determine if this would
be a stable fix or not.

  Luis

Re: [PATCH v3 1/1] mac80211: tell driver when dtim change detected

[Guy, Wey-Yi]

On Fri, 2010-01-22 at 15:44 -0800, Luis R. Rodriguez wrote:
> On Fri, Jan 22, 2010 at 11:46 AM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > On Fri, 2010-01-22 at 11:20 -0800, Luis R. Rodriguez wrote:
> >
> >> > Previously, we would switch the channel completely to the new operating
> >> > channel before even probing the AP. That way, we would virtually always
> >> > receive a beacon from the new AP between the time we started the
> >> > association process (probe,auth,assoc) and configuring the driver.
> >> >
> >> > Now with the new changes that use the off-channel work, we may return to
> >> > the old "operating" channel, which may be no particular channel, between
> >> > all these steps. Thus, if there's no beacon between any of probe
> >> > request/response, auth "request"/"response", assoc request/response, we
> >> > never get one, and this situation happens.
> >> >
> >> > I see two solutions, apart from this special-case patch fixing
> >> >
> >> > First, we could go back to the original behaviour if we have just one
> >> > virtual interface. But that still leaves us with the race, we might do
> >> > all three frame exchanges within a beacon interval and still miss the
> >> > beacon, we just tend to not do that and get a beacon.
> >>
> >> Curious, what symptoms were seen when the dtim was not propagated
> >> prior to association, did the STA just not wake up for the right dtim
> >> interval when in PS mode?
> >
> > Yes, I don't really know exactly what happens,
> 
> OK -- Wey, can you elaborate a little as to how you found this and
> what you were seeing? Does the device not go into PS mode at all?

We are doing Power consumption test with different DTIM value, the test
is based on test scripts, so we disable the NetworkManager to make sure
we can control which AP we need to associated with. During the test, we
notice when AP change it DTIM value, a lot time STA will used the old
DTIM value in Power Save mode which cause problem.

We believe the correct fix is mac80211 should wait for both probe
response and beacon arrived before start the association process.

Hope this explain what we found.

> 
> >  but if you look at
> > Wey-Yi's patch you'll see most of the problem was that we calculated the
> > maximum powersaving interval wrong.
> 
> I see..
> 
> >> Wouldn't we get the dtim interval on
> >> eventual beacons later or do we disregard all that information after
> >> associated?
> >
> > As you can see from the patch, we currently disregard any "update" in
> > that information -- this patch changes it.
> 
> Got it.
> 
> >> If so what if the AP changes the dtim interval?
> >
> > I believe the AP is not allowed to do that without turning off and back
> > on completely.
> 
> OK. Still curious, if in practice would funky APs ever tune DTIM
> dynamically and not kick you off, and if they do is it fair to ignore
> that and blame the AP?
> 
> >> I take it this can easily be reproduced with a long beacon interval?
> >
> > Easier, yes. The easiest way to reproduce it is to associate using just
> > "iw", no NM or wpa_supplicant -- make sure the scan list is empty before
> > you associate -- and with a long beacon interval. We then scan, but that
> > will only give us a probe response with high probability, and then we do
> > all the switching as I explained and we don't ever get a beacon. If you
> > use NM it may scan more often and increase the probability of us having
> > a beacon at some point in time.
> 
> OK thanks.
> 
> >> > The other solution I see is that we add a new step before or after the
> >> > direct probe step, which would just be "wait for a beacon". This would
> >> > ensure we have both probe and beacon information always ready. It would
> >> > also ensure we have both probe and beacon info for our new userspace
> >> > reporting of that.
> >>
> >> This seems cleaner.
> >
> > I agree. And since we already keep track of this since 34a6eddb, we
> > should be able to fairly easily determine whether we still need probe
> > response or beacon, and only do either the direct probe or wait for
> > beacon step.
> 
>   Luis

Re: [PATCH v3 1/1] mac80211: tell driver when dtim change detected

[Luis R. Rodriguez]

On Fri, Jan 22, 2010 at 4:11 PM, Guy, Wey-Yi <wey-yi.w.guy@intel.com> wrote:
> On Fri, 2010-01-22 at 15:44 -0800, Luis R. Rodriguez wrote:
>> On Fri, Jan 22, 2010 at 11:46 AM, Johannes Berg
>> <johannes@sipsolutions.net> wrote:
>> > On Fri, 2010-01-22 at 11:20 -0800, Luis R. Rodriguez wrote:
>> >
>> >> > Previously, we would switch the channel completely to the new operating
>> >> > channel before even probing the AP. That way, we would virtually always
>> >> > receive a beacon from the new AP between the time we started the
>> >> > association process (probe,auth,assoc) and configuring the driver.
>> >> >
>> >> > Now with the new changes that use the off-channel work, we may return to
>> >> > the old "operating" channel, which may be no particular channel, between
>> >> > all these steps. Thus, if there's no beacon between any of probe
>> >> > request/response, auth "request"/"response", assoc request/response, we
>> >> > never get one, and this situation happens.
>> >> >
>> >> > I see two solutions, apart from this special-case patch fixing
>> >> >
>> >> > First, we could go back to the original behaviour if we have just one
>> >> > virtual interface. But that still leaves us with the race, we might do
>> >> > all three frame exchanges within a beacon interval and still miss the
>> >> > beacon, we just tend to not do that and get a beacon.
>> >>
>> >> Curious, what symptoms were seen when the dtim was not propagated
>> >> prior to association, did the STA just not wake up for the right dtim
>> >> interval when in PS mode?
>> >
>> > Yes, I don't really know exactly what happens,
>>
>> OK -- Wey, can you elaborate a little as to how you found this and
>> what you were seeing? Does the device not go into PS mode at all?
>
> We are doing Power consumption test with different DTIM value, the test
> is based on test scripts, so we disable the NetworkManager to make sure
> we can control which AP we need to associated with. During the test, we
> notice when AP change it DTIM value, a lot time STA will used the old
> DTIM value in Power Save mode which cause problem.

Ah I see. What if there was no old dtim value before?

> We believe the correct fix is mac80211 should wait for both probe
> response and beacon arrived before start the association process.
>
> Hope this explain what we found.

Indeed, thanks a lot.

  Luis

Re: [PATCH v3 1/1] mac80211: tell driver when dtim change detected

[Guy, Wey-Yi]

On Fri, 2010-01-22 at 16:23 -0800, Luis R. Rodriguez wrote:
> On Fri, Jan 22, 2010 at 4:11 PM, Guy, Wey-Yi <wey-yi.w.guy@intel.com> wrote:
> > On Fri, 2010-01-22 at 15:44 -0800, Luis R. Rodriguez wrote:
> >> On Fri, Jan 22, 2010 at 11:46 AM, Johannes Berg
> >> <johannes@sipsolutions.net> wrote:
> >> > On Fri, 2010-01-22 at 11:20 -0800, Luis R. Rodriguez wrote:
> >> >
> >> >> > Previously, we would switch the channel completely to the new operating
> >> >> > channel before even probing the AP. That way, we would virtually always
> >> >> > receive a beacon from the new AP between the time we started the
> >> >> > association process (probe,auth,assoc) and configuring the driver.
> >> >> >
> >> >> > Now with the new changes that use the off-channel work, we may return to
> >> >> > the old "operating" channel, which may be no particular channel, between
> >> >> > all these steps. Thus, if there's no beacon between any of probe
> >> >> > request/response, auth "request"/"response", assoc request/response, we
> >> >> > never get one, and this situation happens.
> >> >> >
> >> >> > I see two solutions, apart from this special-case patch fixing
> >> >> >
> >> >> > First, we could go back to the original behaviour if we have just one
> >> >> > virtual interface. But that still leaves us with the race, we might do
> >> >> > all three frame exchanges within a beacon interval and still miss the
> >> >> > beacon, we just tend to not do that and get a beacon.
> >> >>
> >> >> Curious, what symptoms were seen when the dtim was not propagated
> >> >> prior to association, did the STA just not wake up for the right dtim
> >> >> interval when in PS mode?
> >> >
> >> > Yes, I don't really know exactly what happens,
> >>
> >> OK -- Wey, can you elaborate a little as to how you found this and
> >> what you were seeing? Does the device not go into PS mode at all?
> >
> > We are doing Power consumption test with different DTIM value, the test
> > is based on test scripts, so we disable the NetworkManager to make sure
> > we can control which AP we need to associated with. During the test, we
> > notice when AP change it DTIM value, a lot time STA will used the old
> > DTIM value in Power Save mode which cause problem.
> 
> Ah I see. What if there was no old dtim value before?

I am not sure, I believe the default is "1", but I might be wrong

> 
> > We believe the correct fix is mac80211 should wait for both probe
> > response and beacon arrived before start the association process.
> >
> > Hope this explain what we found.
> 
> Indeed, thanks a lot.
> 
>   Luis

Re: [PATCH v3 1/1] mac80211: tell driver when dtim change detected

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 635 bytes --]

On Fri, 2010-01-22 at 16:22 -0800, Guy, Wey-Yi wrote:

> > > We are doing Power consumption test with different DTIM value, the test
> > > is based on test scripts, so we disable the NetworkManager to make sure
> > > we can control which AP we need to associated with. During the test, we
> > > notice when AP change it DTIM value, a lot time STA will used the old
> > > DTIM value in Power Save mode which cause problem.
> > 
> > Ah I see. What if there was no old dtim value before?
> 
> I am not sure, I believe the default is "1", but I might be wrong

Yes, since 0 is nonsensical, we would use 1 instead.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v3 1/1] mac80211: tell driver when dtim change detected

[Luis R. Rodriguez]

On Sat, Jan 23, 2010 at 4:46 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Fri, 2010-01-22 at 16:22 -0800, Guy, Wey-Yi wrote:
>
>> > > We are doing Power consumption test with different DTIM value, the test
>> > > is based on test scripts, so we disable the NetworkManager to make sure
>> > > we can control which AP we need to associated with. During the test, we
>> > > notice when AP change it DTIM value, a lot time STA will used the old
>> > > DTIM value in Power Save mode which cause problem.
>> >
>> > Ah I see. What if there was no old dtim value before?
>>
>> I am not sure, I believe the default is "1", but I might be wrong
>
> Yes, since 0 is nonsensical, we would use 1 instead.

OK -- it seems we have a consensus waiting for a beacon would be good,
but it would be good to have something in for stable. Is the patch Guy
posted acceptable for that purpose? That is, merge this for now until
we get beacon wait implemented prior to association.

  Luis

Re: [PATCH v3 1/1] mac80211: tell driver when dtim change detected

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 927 bytes --]

On Mon, 2010-01-25 at 10:18 -0800, Luis R. Rodriguez wrote:

> > Yes, since 0 is nonsensical, we would use 1 instead.
> 
> OK -- it seems we have a consensus waiting for a beacon would be good,
> but it would be good to have something in for stable. Is the patch Guy
> posted acceptable for that purpose? That is, merge this for now until
> we get beacon wait implemented prior to association.

Not sure we even need it in stable that badly? It seems it only hurts
power consumption somewhat. And I just implemented waiting today too.

Although Bob has a point, we could change the way we pass the DTIM
period to the driver, only pass it in the powersave callbacks, and then
we could associate without it and just enable powersave only after
receiving a beacon. That assumes that drivers don't use it for anything
else though, of course, but I don't see anything else you could use it
for really.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v3 1/1] mac80211: tell driver when dtim change detected

[Luis R. Rodriguez]

On Mon, Jan 25, 2010 at 10:33 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Mon, 2010-01-25 at 10:18 -0800, Luis R. Rodriguez wrote:
>
>> > Yes, since 0 is nonsensical, we would use 1 instead.
>>
>> OK -- it seems we have a consensus waiting for a beacon would be good,
>> but it would be good to have something in for stable. Is the patch Guy
>> posted acceptable for that purpose? That is, merge this for now until
>> we get beacon wait implemented prior to association.
>
> Not sure we even need it in stable that badly? It seems it only hurts
> power consumption

Good point, we'd just be waking up at every beacon if power save is
enabled since the default is indeed 1.

> somewhat.

Depends how common this is. I am not sure what the usual DTIM value is
either though, but if its not 1 and this is indeed very common that'd
be a bit sloppy for stable.

> And I just implemented waiting today too.

I saw, you're the fucking man.

> Although Bob has a point, we could change the way we pass the DTIM
> period to the driver, only pass it in the powersave callbacks, and then
> we could associate without it and just enable powersave only after
> receiving a beacon. That assumes that drivers don't use it for anything
> else though, of course, but I don't see anything else you could use it
> for really.

Neat -- but that would mean doing the same for other beacon-sync IE
information. What other stuff goes out in beacons that does not go
into probe responses? I think I read a thread the other day about some
WPS information.

  Luis

Re: [PATCH v3 1/1] mac80211: tell driver when dtim change detected

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 921 bytes --]

On Mon, 2010-01-25 at 11:55 -0800, Luis R. Rodriguez wrote:

> > Although Bob has a point, we could change the way we pass the DTIM
> > period to the driver, only pass it in the powersave callbacks, and then
> > we could associate without it and just enable powersave only after
> > receiving a beacon. That assumes that drivers don't use it for anything
> > else though, of course, but I don't see anything else you could use it
> > for really.
> 
> Neat -- but that would mean doing the same for other beacon-sync IE
> information. What other stuff goes out in beacons that does not go
> into probe responses? I think I read a thread the other day about some
> WPS information.

Ah, yes, but that's a userspace thing -- and before association too.

I'll need to analyse this in more detail, and also what drivers do.

Kalle, could wl12x1 deal with getting the DTIM only when enabling PS?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v3 1/1] mac80211: tell driver when dtim change detected

[Kalle Valo]

Johannes Berg <johannes@sipsolutions.net> writes:

> I'll need to analyse this in more detail, and also what drivers do.
>
> Kalle, could wl12x1 deal with getting the DTIM only when enabling PS?

Yes, wl1251 sets dtim with wl1251_acx_wr_tbtt_and_dtim() and it should
be enough to set it before enabling PS.

-- 
Kalle Valo

Re: [PATCH v3 1/1] mac80211: tell driver when dtim change detected

[Jouni Malinen]

On Fri, Jan 22, 2010 at 03:44:41PM -0800, Luis R. Rodriguez wrote:

> OK. Still curious, if in practice would funky APs ever tune DTIM
> dynamically and not kick you off, and if they do is it fair to ignore
> that and blame the AP?

As far as I can tell, the standard requires you to restart the BSS if
you change the DTIM value. I don't think there is explicit requirement
for the AP to send you a deauthentication or disassociation frame,
though, but I would expect the association to be lost if the AP is
compliant with the standard.

Anyway, it is not like all APs really follow the standard requirements
in all cases, so I would not be to surprised to hear about an AP that
changes the DTIM value more dynamically. Anyway, that is just asking for
problems due to the way a specific timestamps are defined to be used for
the beacons that are followed by buffered multicast/broadcast frames.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH v3 1/1] mac80211: tell driver when dtim change detected

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 529 bytes --]

On Mon, 2010-01-25 at 10:32 -0800, Jouni Malinen wrote:

> Anyway, that is just asking for
> problems due to the way a specific timestamps are defined to be used for
> the beacons that are followed by buffered multicast/broadcast frames.

That's actually an interesting point -- does anyone implement that
properly, or is that just a side note in the standard nobody even reads?
We certainly don't in mac80211 since we don't have that level of
synchronisation between the TSF and the beacon get function.

johannes


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v3 1/1] mac80211: tell driver when dtim change detected

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 837 bytes --]

On Mon, 2010-01-25 at 19:36 +0100, Johannes Berg wrote:
> On Mon, 2010-01-25 at 10:32 -0800, Jouni Malinen wrote:
> 
> > Anyway, that is just asking for
> > problems due to the way a specific timestamps are defined to be used for
> > the beacons that are followed by buffered multicast/broadcast frames.
> 
> That's actually an interesting point -- does anyone implement that
> properly, or is that just a side note in the standard nobody even reads?
> We certainly don't in mac80211 since we don't have that level of
> synchronisation between the TSF and the beacon get function.

And, in fact, the DTIM count is kinda useless if one was to rely on that
timing requirement, so I have a feeling that the intention of the people
who wrote this bit of the standard wasn't to actually impose such a
requirement.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v3 1/1] mac80211: tell driver when dtim change detected

[Kalle Valo]

Johannes Berg <johannes@sipsolutions.net> writes:

> However, I think it's indicative of a bigger problem. I gave it some
> thought, and came to the conclusion that it previously didn't happen
> because we always won the race. Let me explain.

One more reason why need to receive the beacon is synchronisation to
the beacon for power save. For example wl1251 and wl1271 want to
receive one beacon for synchronisation purposes before some of the
commands are executed. It would be really nice if mac80211 would take
care of that.

> First, we could go back to the original behaviour if we have just one
> virtual interface. 

I think this is good to have. Just to keep it simple for devices like
wl1251 and wl1271.

> But that still leaves us with the race, we might do all three frame
> exchanges within a beacon interval and still miss the beacon, we
> just tend to not do that and get a beacon.
>
> The other solution I see is that we add a new step before or after the
> direct probe step, which would just be "wait for a beacon". This would
> ensure we have both probe and beacon information always ready. It would
> also ensure we have both probe and beacon info for our new userspace
> reporting of that.

I like this. If only the bssid would be provided to the driver before
the association process, both wl1251 and wl1271 would not have to
worry about firmware beacon synchronisation.

-- 
Kalle Valo

Re: [PATCH v3 1/1] mac80211: tell driver when dtim change detected

[Jouni Malinen]

On Fri, Jan 22, 2010 at 08:03:01PM +0100, Johannes Berg wrote:
> The other solution I see is that we add a new step before or after the
> direct probe step, which would just be "wait for a beacon". This would
> ensure we have both probe and beacon information always ready. It would
> also ensure we have both probe and beacon info for our new userspace
> reporting of that.

I'm somewhat concerned about this as an unconditional change for our
association process due to the extra latency it adds. At minimum, I
would like to see a driver flag that can be used to indicate that such
behavior is really required and skip the extra wait if the driver does
not need the Beacon frame before association.

For most cases, I would also assume it should be possible to update the
PS settings after having received the first Beacon frame after
association, but if that is not enough, allowing the driver to request
mac80211 to wait with association sounds reasonable. I just don't want
to see additional 50 msec or so (or much worse if the AP is configured
with insanely long beacon interval) delay popping up with every
association by default..

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH v3 1/1] mac80211: tell driver when dtim change detected

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1472 bytes --]

On Mon, 2010-01-25 at 10:35 -0800, Jouni Malinen wrote:
> On Fri, Jan 22, 2010 at 08:03:01PM +0100, Johannes Berg wrote:
> > The other solution I see is that we add a new step before or after the
> > direct probe step, which would just be "wait for a beacon". This would
> > ensure we have both probe and beacon information always ready. It would
> > also ensure we have both probe and beacon info for our new userspace
> > reporting of that.
> 
> I'm somewhat concerned about this as an unconditional change for our
> association process due to the extra latency it adds. At minimum, I
> would like to see a driver flag that can be used to indicate that such
> behavior is really required and skip the extra wait if the driver does
> not need the Beacon frame before association.
> 
> For most cases, I would also assume it should be possible to update the
> PS settings after having received the first Beacon frame after
> association, but if that is not enough, allowing the driver to request
> mac80211 to wait with association sounds reasonable. I just don't want
> to see additional 50 msec or so (or much worse if the AP is configured
> with insanely long beacon interval) delay popping up with every
> association by default..

Yeah that's a good point, I wondered about that too. I'll look into this
in more detail and see if we can tell the driver about the DTIM period
only with CONF_PS and enable that only after a beacon.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v3 1/1] mac80211: tell driver when dtim change detected

[Guy, Wey-Yi]

On Mon, 2010-01-25 at 12:11 -0800, Johannes Berg wrote:
> On Mon, 2010-01-25 at 10:35 -0800, Jouni Malinen wrote:
> > On Fri, Jan 22, 2010 at 08:03:01PM +0100, Johannes Berg wrote:
> > > The other solution I see is that we add a new step before or after the
> > > direct probe step, which would just be "wait for a beacon". This would
> > > ensure we have both probe and beacon information always ready. It would
> > > also ensure we have both probe and beacon info for our new userspace
> > > reporting of that.
> > 
> > I'm somewhat concerned about this as an unconditional change for our
> > association process due to the extra latency it adds. At minimum, I
> > would like to see a driver flag that can be used to indicate that such
> > behavior is really required and skip the extra wait if the driver does
> > not need the Beacon frame before association.
> > 
> > For most cases, I would also assume it should be possible to update the
> > PS settings after having received the first Beacon frame after
> > association, but if that is not enough, allowing the driver to request
> > mac80211 to wait with association sounds reasonable. I just don't want
> > to see additional 50 msec or so (or much worse if the AP is configured
> > with insanely long beacon interval) delay popping up with every
> > association by default..
> 
> Yeah that's a good point, I wondered about that too. I'll look into this
> in more detail and see if we can tell the driver about the DTIM period
> only with CONF_PS and enable that only after a beacon.
> 
I really like the idea of using CONF_PS which will not cause any delay
for association, plus the only STA need to know DTIM is the station in
power save mode.