2013-12-26 13:42:25

by Chaitanya Tata

Subject: [PATCH] mac80211: Return untuned timestamp in case of invalid bitrates

In case the driver sends MCS9 and doesn't advertise CBW
(or) advertises 20MHz bandwidth, return the timestamp As is,
instead of returning 0 as timestamp.

Signed-off-by: Chaitanya T K <[email protected]>
---
net/mac80211/util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/util.c b/net/mac80211/util.c index 06265d7..bc92b25 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -2265,7 +2265,7 @@ u64 ieee80211_calculate_rx_timestamp(struct ieee80211_local *local,
if (WARN_ONCE(!rate,
"Invalid bitrate: flags=0x%x, idx=%d, vht_nss=%d\n",
status->flag, status->rate_idx, status->vht_nss))
- return 0;
+ return ts;

/* rewind from end of MPDU */
if (status->flag & RX_FLAG_MACTIME_END)

2014-01-06 15:04:27

by Johannes Berg

Subject: Re: [PATCH] mac80211: Return untuned timestamp in case of invalid bitrates

On Thu, 2013-12-26 at 13:42 +0000, Chaitanya Tata wrote:
> In case the driver sends MCS9 and doesn't advertise CBW
> (or) advertises 20MHz bandwidth, return the timestamp As is,
> instead of returning 0 as timestamp.
>
> Signed-off-by: Chaitanya T K <[email protected]>
> ---
> net/mac80211/util.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/mac80211/util.c b/net/mac80211/util.c index 06265d7..bc92b25 100644
> --- a/net/mac80211/util.c
> +++ b/net/mac80211/util.c
> @@ -2265,7 +2265,7 @@ u64 ieee80211_calculate_rx_timestamp(struct ieee80211_local *local,
> if (WARN_ONCE(!rate,
> "Invalid bitrate: flags=0x%x, idx=%d, vht_nss=%d\n",
> status->flag, status->rate_idx, status->vht_nss))
> - return 0;
> + return ts;

I think this is wrong - we've already warned, we know we have no usable
timestamp, and this should never happen anyway.

johannes

2014-01-06 15:07:52

by Krishna Chaitanya

Subject: Re: [PATCH] mac80211: Return untuned timestamp in case of invalid bitrates

On Mon, Jan 6, 2014 at 8:34 PM, Johannes Berg <[email protected]> wrote:
> On Thu, 2013-12-26 at 13:42 +0000, Chaitanya Tata wrote:
>> In case the driver sends MCS9 and doesn't advertise CBW
>> (or) advertises 20MHz bandwidth, return the timestamp As is,
>> instead of returning 0 as timestamp.
>>
>> Signed-off-by: Chaitanya T K <[email protected]>
>> ---
>> net/mac80211/util.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/mac80211/util.c b/net/mac80211/util.c index 06265d7..bc92b25 100644
>> --- a/net/mac80211/util.c
>> +++ b/net/mac80211/util.c
>> @@ -2265,7 +2265,7 @@ u64 ieee80211_calculate_rx_timestamp(struct ieee80211_local *local,
>> if (WARN_ONCE(!rate,
>> "Invalid bitrate: flags=0x%x, idx=%d, vht_nss=%d\n",
>> status->flag, status->rate_idx, status->vht_nss))
>> - return 0;
>> + return ts;
>
> I think this is wrong - we've already warned, we know we have no usable
> timestamp, and this should never happen anyway.

Timestamp is just off by packet length (as we dont have proper MCS rates),
but still we can use it as un-rewinded timestamp.

2014-01-06 15:14:36

by Johannes Berg

Subject: Re: [PATCH] mac80211: Return untuned timestamp in case of invalid bitrates

On Mon, 2014-01-06 at 20:37 +0530, Krishna Chaitanya wrote:
> On Mon, Jan 6, 2014 at 8:34 PM, Johannes Berg <[email protected]> wrote:
> > On Thu, 2013-12-26 at 13:42 +0000, Chaitanya Tata wrote:
> >> In case the driver sends MCS9 and doesn't advertise CBW
> >> (or) advertises 20MHz bandwidth, return the timestamp As is,
> >> instead of returning 0 as timestamp.
> >>
> >> Signed-off-by: Chaitanya T K <[email protected]>
> >> ---
> >> net/mac80211/util.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/net/mac80211/util.c b/net/mac80211/util.c index 06265d7..bc92b25 100644
> >> --- a/net/mac80211/util.c
> >> +++ b/net/mac80211/util.c
> >> @@ -2265,7 +2265,7 @@ u64 ieee80211_calculate_rx_timestamp(struct ieee80211_local *local,
> >> if (WARN_ONCE(!rate,
> >> "Invalid bitrate: flags=0x%x, idx=%d, vht_nss=%d\n",
> >> status->flag, status->rate_idx, status->vht_nss))
> >> - return 0;
> >> + return ts;
> >
> > I think this is wrong - we've already warned, we know we have no usable
> > timestamp, and this should never happen anyway.
>
> Timestamp is just off by packet length (as we dont have proper MCS rates),
> but still we can use it as un-rewinded timestamp.

You've just hit a WARN_ONCE()! This case really shouldn't happen, and
you should change the driver to not hit it. If you don't have a valid RX
bitrate, this is probably the least of your worries.

johannes