2012-03-13 13:04:17

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 1/2] brmc80211: dont use jiffies for BSS TSF

From: Johannes Berg <[email protected]>

The cfg80211_inform_bss() timestamp argument is
intended to be the TSF, not any form of host
timestamp.

Signed-off-by: Johannes Berg <[email protected]>
---
drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)

--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c 2012-03-10 09:17:00.000000000 +0100
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c 2012-03-13 13:45:43.000000000 +0100
@@ -2003,7 +2003,6 @@ static s32 brcmf_inform_single_bss(struc
s32 err = 0;
u16 channel;
u32 freq;
- u64 notify_timestamp;
u16 notify_capability;
u16 notify_interval;
u8 *notify_ie;
@@ -2026,7 +2025,6 @@ static s32 brcmf_inform_single_bss(struc
freq = ieee80211_channel_to_frequency(channel, band->band);
notify_channel = ieee80211_get_channel(wiphy, freq);

- notify_timestamp = jiffies_to_msecs(jiffies)*1000; /* uSec */
notify_capability = le16_to_cpu(bi->capability);
notify_interval = le16_to_cpu(bi->beacon_period);
notify_ie = (u8 *)bi + le16_to_cpu(bi->ie_offset);
@@ -2040,10 +2038,9 @@ static s32 brcmf_inform_single_bss(struc
WL_CONN("Capability: %X\n", notify_capability);
WL_CONN("Beacon interval: %d\n", notify_interval);
WL_CONN("Signal: %d\n", notify_signal);
- WL_CONN("notify_timestamp: %#018llx\n", notify_timestamp);

bss = cfg80211_inform_bss(wiphy, notify_channel, (const u8 *)bi->BSSID,
- notify_timestamp, notify_capability, notify_interval, notify_ie,
+ 0, notify_capability, notify_interval, notify_ie,
notify_ielen, notify_signal, GFP_KERNEL);

if (!bss)
@@ -2098,7 +2095,6 @@ static s32 wl_inform_ibss(struct brcmf_c
s32 err = 0;
u16 channel;
u32 freq;
- u64 notify_timestamp;
u16 notify_capability;
u16 notify_interval;
u8 *notify_ie;
@@ -2134,7 +2130,6 @@ static s32 wl_inform_ibss(struct brcmf_c
freq = ieee80211_channel_to_frequency(channel, band->band);
notify_channel = ieee80211_get_channel(wiphy, freq);

- notify_timestamp = jiffies_to_msecs(jiffies)*1000; /* uSec */
notify_capability = le16_to_cpu(bi->capability);
notify_interval = le16_to_cpu(bi->beacon_period);
notify_ie = (u8 *)bi + le16_to_cpu(bi->ie_offset);
@@ -2145,10 +2140,9 @@ static s32 wl_inform_ibss(struct brcmf_c
WL_CONN("capability: %X\n", notify_capability);
WL_CONN("beacon interval: %d\n", notify_interval);
WL_CONN("signal: %d\n", notify_signal);
- WL_CONN("notify_timestamp: %#018llx\n", notify_timestamp);

bss = cfg80211_inform_bss(wiphy, notify_channel, bssid,
- notify_timestamp, notify_capability, notify_interval,
+ 0, notify_capability, notify_interval,
notify_ie, notify_ielen, notify_signal, GFP_KERNEL);

if (!bss) {




2012-03-13 18:33:08

by Franky Lin

[permalink] [raw]
Subject: Re: [PATCH 1/2] brmc80211: dont use jiffies for BSS TSF

On 03/13/2012 05:57 AM, Johannes Berg wrote:
> From: Johannes Berg<[email protected]>
>
> The cfg80211_inform_bss() timestamp argument is
> intended to be the TSF, not any form of host
> timestamp.
>
> Signed-off-by: Johannes Berg<[email protected]>

Thanks Johannes.

Acked-by: Franky Lin <[email protected]>


2014-12-22 13:14:39

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/2] brmc80211: dont use jiffies for BSS TSF

On Mon, 2014-12-22 at 14:13 +0100, Arend van Spriel wrote:
> On 12/22/14 13:58, Johannes Berg wrote:
> > By the way - I know now that the proprietary Broadcom driver has the
> > same bug, to the point where this is apparently getting encoded into the
> > Android framework.
> >
> > I urge you to fix this issue there as well. If an absolute "last
> > updated" timestamp is needed (and "last seen [ms] ago" isn't sufficient)
> > then new API will be needed.
>
> Sorry, seem to have missed the original patches somehow. Guess because
> it says *brmc*80211.

No no - it's a looong time ago and was also applied a long time ago. It
just reared it's head again in another place.

johannes


2014-12-22 12:58:14

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/2] brmc80211: dont use jiffies for BSS TSF

By the way - I know now that the proprietary Broadcom driver has the
same bug, to the point where this is apparently getting encoded into the
Android framework.

I urge you to fix this issue there as well. If an absolute "last
updated" timestamp is needed (and "last seen [ms] ago" isn't sufficient)
then new API will be needed.

johannes


2014-12-22 13:13:38

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH 1/2] brmc80211: dont use jiffies for BSS TSF

On 12/22/14 13:58, Johannes Berg wrote:
> By the way - I know now that the proprietary Broadcom driver has the
> same bug, to the point where this is apparently getting encoded into the
> Android framework.
>
> I urge you to fix this issue there as well. If an absolute "last
> updated" timestamp is needed (and "last seen [ms] ago" isn't sufficient)
> then new API will be needed.

Sorry, seem to have missed the original patches somehow. Guess because
it says *brmc*80211.

Regards,
Arend

2014-12-24 07:49:57

by Arik Nemtsov

[permalink] [raw]
Subject: Re: [PATCH 1/2] brmc80211: dont use jiffies for BSS TSF

On Mon, Dec 22, 2014 at 3:26 PM, Arend van Spriel <[email protected]> wrote:
> On 12/22/14 14:14, Johannes Berg wrote:
>>
>> On Mon, 2014-12-22 at 14:13 +0100, Arend van Spriel wrote:
>>>
>>> On 12/22/14 13:58, Johannes Berg wrote:
>>>>
>>>> By the way - I know now that the proprietary Broadcom driver has the
>>>> same bug, to the point where this is apparently getting encoded into the
>>>> Android framework.
>>>>
>>>> I urge you to fix this issue there as well. If an absolute "last
>>>> updated" timestamp is needed (and "last seen [ms] ago" isn't sufficient)
>>>> then new API will be needed.
>>>
>>>
>>> Sorry, seem to have missed the original patches somehow. Guess because
>>> it says *brmc*80211.
>>
>>
>> No no - it's a looong time ago and was also applied a long time ago. It
>> just reared it's head again in another place.
>
>
> Found the commit in git log. So you mean the proprietary DHD driver or
> Android bcmdhd still uses host timestamp. I agree it should be fixed, but I
> may need some good arguments to convince my co-workers. So what kind of
> issues are rearing their (ugly) heads? supplicant issues?

Basically the Android framework is using the TSF from the scan results
as a timestamp for when the BSS was seen. In Android LL a new CTS test
was added to verify these values.
This of course means the CTS test fails for every vendor besides BRCM :)

It seems the framework knows it's a driver bug, since in the java code
they explicitly name the variable "tsf", before assigning it to the
scan result timestamp..

Arik

2014-12-22 13:26:54

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH 1/2] brmc80211: dont use jiffies for BSS TSF

On 12/22/14 14:14, Johannes Berg wrote:
> On Mon, 2014-12-22 at 14:13 +0100, Arend van Spriel wrote:
>> On 12/22/14 13:58, Johannes Berg wrote:
>>> By the way - I know now that the proprietary Broadcom driver has the
>>> same bug, to the point where this is apparently getting encoded into the
>>> Android framework.
>>>
>>> I urge you to fix this issue there as well. If an absolute "last
>>> updated" timestamp is needed (and "last seen [ms] ago" isn't sufficient)
>>> then new API will be needed.
>>
>> Sorry, seem to have missed the original patches somehow. Guess because
>> it says *brmc*80211.
>
> No no - it's a looong time ago and was also applied a long time ago. It
> just reared it's head again in another place.

Found the commit in git log. So you mean the proprietary DHD driver or
Android bcmdhd still uses host timestamp. I agree it should be fixed,
but I may need some good arguments to convince my co-workers. So what
kind of issues are rearing their (ugly) heads? supplicant issues?

Regards,
Arend