2013-04-13 10:31:57

by Jonas Gorski

[permalink] [raw]
Subject: [RFC/RFT] mwl8k: don't expose non-standard rates

Do not advertise the non standard rates 22 Mbps and 72 Mbps to mac80211,
they will make it into the probe responses and cause clients checking them
to refuse association, e.g. wpa_supplicant says:

wlan1: hardware does not support required rate 8.0 Mbps

Fix this by removing the rates and adapt shifts accordingly.

This fixes at least association with Android ICS or newer as well as
wpa_supplicant with ath9k to a mwl8k running AP.

Signed-off-by: Jonas Gorski <[email protected]>
---

I'm sending this patch as RFC/RFT because I'm not 100% confident I
caught everything - the length of the rate arrays are hardcoded in
several places, and some use an offset into it to make it even more
confusing.

So please test that I did not break anything.

drivers/net/wireless/mwl8k.c | 29 +++++++++--------------------
1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/mwl8k.c b/drivers/net/wireless/mwl8k.c
index ee1778c..04f9051 100644
--- a/drivers/net/wireless/mwl8k.c
+++ b/drivers/net/wireless/mwl8k.c
@@ -193,10 +193,10 @@ struct mwl8k_priv {
struct rxd_ops *rxd_ops;
struct ieee80211_supported_band band_24;
struct ieee80211_channel channels_24[14];
- struct ieee80211_rate rates_24[14];
+ struct ieee80211_rate rates_24[12];
struct ieee80211_supported_band band_50;
struct ieee80211_channel channels_50[4];
- struct ieee80211_rate rates_50[9];
+ struct ieee80211_rate rates_50[8];
u32 ap_macids_supported;
u32 sta_macids_supported;

@@ -357,7 +357,6 @@ static const struct ieee80211_rate mwl8k_rates_24[] = {
{ .bitrate = 20, .hw_value = 4, },
{ .bitrate = 55, .hw_value = 11, },
{ .bitrate = 110, .hw_value = 22, },
- { .bitrate = 220, .hw_value = 44, },
{ .bitrate = 60, .hw_value = 12, },
{ .bitrate = 90, .hw_value = 18, },
{ .bitrate = 120, .hw_value = 24, },
@@ -366,7 +365,6 @@ static const struct ieee80211_rate mwl8k_rates_24[] = {
{ .bitrate = 360, .hw_value = 72, },
{ .bitrate = 480, .hw_value = 96, },
{ .bitrate = 540, .hw_value = 108, },
- { .bitrate = 720, .hw_value = 144, },
};

static const struct ieee80211_channel mwl8k_channels_50[] = {
@@ -385,7 +383,6 @@ static const struct ieee80211_rate mwl8k_rates_50[] = {
{ .bitrate = 360, .hw_value = 72, },
{ .bitrate = 480, .hw_value = 96, },
{ .bitrate = 540, .hw_value = 108, },
- { .bitrate = 720, .hw_value = 144, },
};

/* Set or get info from Firmware */
@@ -1003,7 +1000,7 @@ mwl8k_rxd_ap_process(void *_rxd, struct ieee80211_rx_status *status,
if (rxd->channel > 14) {
status->band = IEEE80211_BAND_5GHZ;
if (!(status->flag & RX_FLAG_HT))
- status->rate_idx -= 5;
+ status->rate_idx -= 4;
} else {
status->band = IEEE80211_BAND_2GHZ;
}
@@ -1111,7 +1108,7 @@ mwl8k_rxd_sta_process(void *_rxd, struct ieee80211_rx_status *status,
if (rxd->channel > 14) {
status->band = IEEE80211_BAND_5GHZ;
if (!(status->flag & RX_FLAG_HT))
- status->rate_idx -= 5;
+ status->rate_idx -= 4;
} else {
status->band = IEEE80211_BAND_2GHZ;
}
@@ -3080,17 +3077,9 @@ struct mwl8k_cmd_update_set_aid {
static void legacy_rate_mask_to_array(u8 *rates, u32 mask)
{
int i;
- int j;

- /*
- * Clear nonstandard rates 4 and 13.
- */
- mask &= 0x1fef;
-
- for (i = 0, j = 0; i < 14; i++) {
- if (mask & (1 << i))
- rates[j++] = mwl8k_rates_24[i].hw_value;
- }
+ for (i = 0; i < 12; i++)
+ rates[i] = mwl8k_rates_24[i].hw_value;
}

static int
@@ -3972,7 +3961,7 @@ static int mwl8k_cmd_set_new_stn_add(struct ieee80211_hw *hw,
if (hw->conf.chandef.chan->band == IEEE80211_BAND_2GHZ)
rates = sta->supp_rates[IEEE80211_BAND_2GHZ];
else
- rates = sta->supp_rates[IEEE80211_BAND_5GHZ] << 5;
+ rates = sta->supp_rates[IEEE80211_BAND_5GHZ] << 4;
cmd->legacy_rates = cpu_to_le32(rates);
if (sta->ht_cap.ht_supported) {
cmd->ht_rates[0] = sta->ht_cap.mcs.rx_mask[0];
@@ -4407,7 +4396,7 @@ static int mwl8k_cmd_update_stadb_add(struct ieee80211_hw *hw,
if (hw->conf.chandef.chan->band == IEEE80211_BAND_2GHZ)
rates = sta->supp_rates[IEEE80211_BAND_2GHZ];
else
- rates = sta->supp_rates[IEEE80211_BAND_5GHZ] << 5;
+ rates = sta->supp_rates[IEEE80211_BAND_5GHZ] << 4;
legacy_rate_mask_to_array(p->legacy_rates, rates);
memcpy(p->ht_rates, sta->ht_cap.mcs.rx_mask, 16);
p->interop = 1;
@@ -4889,7 +4878,7 @@ mwl8k_bss_info_changed_sta(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
ap_legacy_rates = ap->supp_rates[IEEE80211_BAND_2GHZ];
} else {
ap_legacy_rates =
- ap->supp_rates[IEEE80211_BAND_5GHZ] << 5;
+ ap->supp_rates[IEEE80211_BAND_5GHZ] << 4;
}
memcpy(ap_mcs_rates, ap->ht_cap.mcs.rx_mask, 16);

--
1.7.10.4



2013-04-14 12:13:34

by Jonas Gorski

[permalink] [raw]
Subject: [PATCH] mwl8k: remove nonstandard rate 72 Mbps

This rate causes an overflow in the extended rates IE's data rate field,
with the overflowing bit setting the Basic Rate Set membership. This
results in a bogus 8 Mpbs basic rate, making clients checking them refuse
association.

Since the rate is likely unused anyway (HT will yield better rates between
supporting chips), we can just remove it.

This fixes association from wpa_supplicant and Android 4.x and newer.

Signed-off-by: Jonas Gorski <[email protected]>
---
Changes from RFC -> v1
* only remove 72 Mbps, as that's the problematic one
* reword everything, add real cause

drivers/net/wireless/mwl8k.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/mwl8k.c b/drivers/net/wireless/mwl8k.c
index ee1778c..6820fce 100644
--- a/drivers/net/wireless/mwl8k.c
+++ b/drivers/net/wireless/mwl8k.c
@@ -193,10 +193,10 @@ struct mwl8k_priv {
struct rxd_ops *rxd_ops;
struct ieee80211_supported_band band_24;
struct ieee80211_channel channels_24[14];
- struct ieee80211_rate rates_24[14];
+ struct ieee80211_rate rates_24[13];
struct ieee80211_supported_band band_50;
struct ieee80211_channel channels_50[4];
- struct ieee80211_rate rates_50[9];
+ struct ieee80211_rate rates_50[8];
u32 ap_macids_supported;
u32 sta_macids_supported;

@@ -366,7 +366,6 @@ static const struct ieee80211_rate mwl8k_rates_24[] = {
{ .bitrate = 360, .hw_value = 72, },
{ .bitrate = 480, .hw_value = 96, },
{ .bitrate = 540, .hw_value = 108, },
- { .bitrate = 720, .hw_value = 144, },
};

static const struct ieee80211_channel mwl8k_channels_50[] = {
@@ -385,7 +384,6 @@ static const struct ieee80211_rate mwl8k_rates_50[] = {
{ .bitrate = 360, .hw_value = 72, },
{ .bitrate = 480, .hw_value = 96, },
{ .bitrate = 540, .hw_value = 108, },
- { .bitrate = 720, .hw_value = 144, },
};

/* Set or get info from Firmware */
@@ -3083,11 +3081,11 @@ static void legacy_rate_mask_to_array(u8 *rates, u32 mask)
int j;

/*
- * Clear nonstandard rates 4 and 13.
+ * Clear nonstandard rate 4.
*/
mask &= 0x1fef;

- for (i = 0, j = 0; i < 14; i++) {
+ for (i = 0, j = 0; i < 13; i++) {
if (mask & (1 << i))
rates[j++] = mwl8k_rates_24[i].hw_value;
}
--
1.7.10.4


2013-04-13 18:59:40

by Adrian Chadd

[permalink] [raw]
Subject: Re: [RFC/RFT] mwl8k: don't expose non-standard rates

On 13 April 2013 09:18, Johannes Berg <[email protected]> wrote:

>> Are these "turbo" mode rates? ie, 40MHz wide channels with pre-11n
>> rates on them?
>
> 22 actually *is* a standard rate, it's just that almost nobody
> implements it.

Well, is it the 20mhz rate or a 40mhz turbo rate?

> The original problem seems a bit strange though, seems those should just
> not be marked basic?

Again, if its a custom rate for 11a/11g 40MHz static turbo style
configurations, it should only be exposed when the AP is running in
that particular mode.

And yes, if the hardware does support it, then it should really only
not be configured for basic rates. That way mwl8k<->mwl8k hardware can
take advantage of it when those stations are talking to each other.

(Yeah, I come from a world where FreeBSD users still want to run
Turbo/Static-40MHz operation with things like fast frames and turbo
rates.. damned legacy hardware. :-)



Adrian

2013-04-13 22:08:08

by Felix Fietkau

[permalink] [raw]
Subject: Re: [RFC/RFT] mwl8k: don't expose non-standard rates

On 2013-04-13 8:59 PM, Adrian Chadd wrote:
> On 13 April 2013 09:18, Johannes Berg <[email protected]> wrote:
>
>>> Are these "turbo" mode rates? ie, 40MHz wide channels with pre-11n
>>> rates on them?
>>
>> 22 actually *is* a standard rate, it's just that almost nobody
>> implements it.
>
> Well, is it the 20mhz rate or a 40mhz turbo rate?
>
>> The original problem seems a bit strange though, seems those should just
>> not be marked basic?
>
> Again, if its a custom rate for 11a/11g 40MHz static turbo style
> configurations, it should only be exposed when the AP is running in
> that particular mode.
>
> And yes, if the hardware does support it, then it should really only
> not be configured for basic rates. That way mwl8k<->mwl8k hardware can
> take advantage of it when those stations are talking to each other.
>
> (Yeah, I come from a world where FreeBSD users still want to run
> Turbo/Static-40MHz operation with things like fast frames and turbo
> rates.. damned legacy hardware. :-)
I really don't think this is a 40 MHz thing, the driver does not seem to
have any concept of a non-standard legacy 40 MHz mode. I think they
simply added different modulation for the 72 MBit/s rate.
Keeping this rate doesn't even make sense for mwl8k<->mwl8k, as it's a
non-standard legacy rate, whereas the device can do much better with
plain 802.11n :)

- Felix

2013-04-14 10:49:35

by Jonas Gorski

[permalink] [raw]
Subject: Re: [RFC/RFT] mwl8k: don't expose non-standard rates

On 13 April 2013 18:18, Johannes Berg <[email protected]> wrote:
> On Sat, 2013-04-13 at 07:29 -0700, Adrian Chadd wrote:
>> Hi,
>>
>> On 13 April 2013 03:31, Jonas Gorski <[email protected]> wrote:
>> > Do not advertise the non standard rates 22 Mbps and 72 Mbps to mac80211,
>> > they will make it into the probe responses and cause clients checking them
>> > to refuse association, e.g. wpa_supplicant says:
>>
>> Are these "turbo" mode rates? ie, 40MHz wide channels with pre-11n
>> rates on them?
>
> 22 actually *is* a standard rate, it's just that almost nobody
> implements it.
>
> The original problem seems a bit strange though, seems those should just
> not be marked basic?

After looking at the standard, it became clear that it is only the 72
Mbps rate that's creating the havoc:

It will be put in the extended rates, but since 72 * 2 is 144, it will
overflow the data rate field and set the basic rate flag/MSB, causing
the bogus 8 Mbps "basic" rate.

I'll respin this patch with just removing the 72 Mbps rate.


Jonas

2013-04-15 07:53:11

by Jonas Gorski

[permalink] [raw]
Subject: Re: [PATCH] mwl8k: remove nonstandard rate 72 Mbps

On Mon, 15 Apr 2013 12:08:38 +0530
Yogesh Ashok Powar <[email protected]> wrote:

> On Sun, Apr 14, 2013 at 05:11:58AM -0700, Jonas Gorski wrote:
> > This rate causes an overflow in the extended rates IE's data rate
> > field, with the overflowing bit setting the Basic Rate Set
> > membership. This results in a bogus 8 Mpbs basic rate, making
> > clients checking them refuse association.
> >
> > Since the rate is likely unused anyway (HT will yield better rates
> > between supporting chips), we can just remove it.
> >
> > This fixes association from wpa_supplicant and Android 4.x and
> > newer.
>
> Hi Jonas,
>
> If you are using hostapd, have you tried using 'supported_rates'
> and/or 'basic_rates' being set to correct rates in the hostapd.conf.

Not sure if that helps. The main problem is that this rate is
unadvertisable in Beacons, as (Extended) Supported Rates (802.11-2012
8.4.2.3) encodes the rates as u8, with seven bits for the rate in units
of 500 kb/s, which isn't enough for 72 Mbps; the maximum is 63.5 Mbps.
The result is that it spills into the MSB, marking it as a "Basic Rate"
even though it isn't marked as such by hostapd. It's just that 72 Mbps
=> 0x90 => "8 Mbps, Basic Rate".
This is partially a bug in hostapd, it should probably reject rates not
encodable.
So AFAICT the only way to use it in hostapd is to not use it.

Which makes me wonder how or if this rate is usable at all in 802.11
context, without a proprietary way of telling the other end that this
rate is available, and therefore if it even makes sense to keep.


Jonas

2013-04-15 06:47:36

by Yogesh Ashok Powar

[permalink] [raw]
Subject: Re: [PATCH] mwl8k: remove nonstandard rate 72 Mbps

On Sun, Apr 14, 2013 at 05:11:58AM -0700, Jonas Gorski wrote:
> This rate causes an overflow in the extended rates IE's data rate field,
> with the overflowing bit setting the Basic Rate Set membership. This
> results in a bogus 8 Mpbs basic rate, making clients checking them refuse
> association.
>
> Since the rate is likely unused anyway (HT will yield better rates between
> supporting chips), we can just remove it.
>
> This fixes association from wpa_supplicant and Android 4.x and newer.

Hi Jonas,

If you are using hostapd, have you tried using 'supported_rates' and/or
'basic_rates' being set to correct rates in the hostapd.conf.

Thanks
Yogesh

2013-04-13 16:18:26

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC/RFT] mwl8k: don't expose non-standard rates

On Sat, 2013-04-13 at 07:29 -0700, Adrian Chadd wrote:
> Hi,
>
> On 13 April 2013 03:31, Jonas Gorski <[email protected]> wrote:
> > Do not advertise the non standard rates 22 Mbps and 72 Mbps to mac80211,
> > they will make it into the probe responses and cause clients checking them
> > to refuse association, e.g. wpa_supplicant says:
>
> Are these "turbo" mode rates? ie, 40MHz wide channels with pre-11n
> rates on them?

22 actually *is* a standard rate, it's just that almost nobody
implements it.

The original problem seems a bit strange though, seems those should just
not be marked basic?

johannes


2013-04-13 14:29:23

by Adrian Chadd

[permalink] [raw]
Subject: Re: [RFC/RFT] mwl8k: don't expose non-standard rates

Hi,

On 13 April 2013 03:31, Jonas Gorski <[email protected]> wrote:
> Do not advertise the non standard rates 22 Mbps and 72 Mbps to mac80211,
> they will make it into the probe responses and cause clients checking them
> to refuse association, e.g. wpa_supplicant says:

Are these "turbo" mode rates? ie, 40MHz wide channels with pre-11n
rates on them?



Adrian