2010-05-26 12:46:36

by Juuso Oikarinen

[permalink] [raw]
Subject: [PATCHv2] mac80211: Fix basic rates for created IBSS networks

Currently the mac80211 marks rates 1 and 2 mbps as basic rates for created
ad-hoc networks. To be certifiable, rates 1, 2, 5.5 and 11 need to be marked
basic.

Change this.

Signed-off-by: Juuso Oikarinen <[email protected]>
---
net/mac80211/ibss.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index b2cc1fd..0b8360c 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -529,7 +529,8 @@ static void ieee80211_sta_create_ibss(struct ieee80211_sub_if_data *sdata)
sdata->drop_unencrypted = 0;

__ieee80211_sta_join_ibss(sdata, bssid, sdata->vif.bss_conf.beacon_int,
- ifibss->channel, 3, /* first two are basic */
+ ifibss->channel,
+ 15, /* 1, 2, 5.5 and 11 are basic */
capability, 0);
}

--
1.6.3.3



2010-05-26 12:53:14

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCHv2] mac80211: Fix basic rates for created IBSS networks

On Wed, 2010-05-26 at 15:48 +0300, Juuso Oikarinen wrote:
> Currently the mac80211 marks rates 1 and 2 mbps as basic rates for created
> ad-hoc networks. To be certifiable, rates 1, 2, 5.5 and 11 need to be marked
> basic.
>
> Change this.
>
> Signed-off-by: Juuso Oikarinen <[email protected]>
> ---
> net/mac80211/ibss.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
> index b2cc1fd..0b8360c 100644
> --- a/net/mac80211/ibss.c
> +++ b/net/mac80211/ibss.c
> @@ -529,7 +529,8 @@ static void ieee80211_sta_create_ibss(struct ieee80211_sub_if_data *sdata)
> sdata->drop_unencrypted = 0;
>
> __ieee80211_sta_join_ibss(sdata, bssid, sdata->vif.bss_conf.beacon_int,
> - ifibss->channel, 3, /* first two are basic */
> + ifibss->channel,
> + 15, /* 1, 2, 5.5 and 11 are basic */

It's still wrong for 5 GHz.

We've had this discussion before. Making more rates required will break
compatibility with 11b devices.

What should be done here is add a basic rates parameter to the IBSS join
nl80211 command so that you can decide at runtime which rates to use as
basic.

johannes


2010-05-26 13:27:19

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCHv2] mac80211: Fix basic rates for created IBSS networks

On Wed, 2010-05-26 at 16:13 +0300, Juuso Oikarinen wrote:

> > > diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
> > > index b2cc1fd..0b8360c 100644
> > > --- a/net/mac80211/ibss.c
> > > +++ b/net/mac80211/ibss.c
> > > @@ -529,7 +529,8 @@ static void ieee80211_sta_create_ibss(struct ieee80211_sub_if_data *sdata)
> > > sdata->drop_unencrypted = 0;
> > >
> > > __ieee80211_sta_join_ibss(sdata, bssid, sdata->vif.bss_conf.beacon_int,
> > > - ifibss->channel, 3, /* first two are basic */
> > > + ifibss->channel,
> > > + 15, /* 1, 2, 5.5 and 11 are basic */
> >
> > It's still wrong for 5 GHz.
>
> Yes, it is as wrong for 5GHz as it was before the change.

No it wasn't wrong before the change, and only the comment is now
wrong ...

before the change, 6 and 9 mbps were basic for 5ghz, now it would be
5,9,12 and 18. Neither set actually overlaps with the mandatory rates,
which is a bug I guess?

> > We've had this discussion before. Making more rates required will break
> > compatibility with 11b devices.
>
> I don't understand this. AFAIK also in 11b all these rates are
> mandatory. How come this breaks 11b?

No, only 1mbit is really mandatory in 11b. So I guess our default should
be just "1" rather than "3" or something like that.

> This change is for creating IBSS, not joining. In case of joining, we
> still adhere to the basic rates set by the creator, or that is what I
> understand based on the code.
>
> > What should be done here is add a basic rates parameter to the IBSS join
> > nl80211 command so that you can decide at runtime which rates to use as
> > basic.
>
> Yeah, this is a solution needed latest when 5GHz support is implemented.

Well this hopefully doesn't come as a surprise to you, but mac80211 does
support 5 GHz operation :)

johannes


2010-05-27 08:11:41

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCHv2] mac80211: Fix basic rates for created IBSS networks

On Thu, 2010-05-27 at 08:06 +0300, Juuso Oikarinen wrote:
> On Wed, 2010-05-26 at 16:14 +0200, ext Johannes Berg wrote:
> > TBD
> >
> > Needs to be split up (setting bss_conf and the bss_change flag is a
> > separate bugfix) and tested but this is what I think it should be like.
>
> Yes, I think this would work. Are you planning on working on this patch?

No.

johannes


2010-05-27 12:13:50

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: mark 1, 2, 5.5 and 11Mbps as mandatory rates for 802.11b

On Thu, 2010-05-27 at 14:09 +0200, Benoit Papillault wrote:

> 802.11a = Clause 17, called OFDM
> 802.11b = Clause 15 (called DSSS) & Clause 18 (called HR/DSSS).

Are you sure? Isn't clause 15 pre-amendement?

> Clause 15 defines 1 Mbits and 2 Mbits rates, long PLCP only
> Clause 18 defines 5.5 Mbits and 11 Mbits. Optionally, short PLCP is
> available for 2 Mbits, 5.5 Mbits and 11 Mbits. Since it's optional, such
> short PLCP rates are probably not part of the mandatory rates.
>
> Basic rate set must includes mandatory rates set.

I don't think this is true.

> This is apparently
> what every other AP do today and I indeed see no points in adding more
> rates to the basic rate set other than what is already defined in the
> mandatory rate set (except we cannot rely on this rule when joining an
> existing BSS/IBSS, of course).

Sure, you can add more than say 11b mandatory rates to specifically
exclude non-11g stations from it to improve performance.

johannes


2010-05-27 08:08:47

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: mark 1, 2, 5.5 and 11Mbps as mandatory rates for 802.11b

On Thu, 2010-05-27 at 09:45 +0900, Bruno Randolf wrote:
> IEEE802.11-2007 clause 18.2.3.3 (p640) states that 1, 2, 5.5 & 11 Mbits are
> mandatory rates for what they call High Rate direct sequence spread spectrum
> (HR/DSSS) PHY (with long PLCP).

You're confused. Clause 18 is 11g, we knew that those were mandatory
there, see the code you're modifying.

Clause 15 is 11b, and it shouldn't change.

So NACK.

johannes


2010-05-27 08:12:06

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCHv2] mac80211: Fix basic rates for created IBSS networks

On Thu, 2010-05-27 at 09:41 +0900, Bruno Randolf wrote:

> > + } else {
> > + /*
> > + * If no rates were explicitly configured,
> > + * use the mandatory rate set for 11b or
> > + * 11a for maximum compatibility.
> > + */
> > + struct ieee80211_supported_band *sband =
> > + wiphy->bands[ibss.channel->band];
> > + int j;
> > + u32 flag = ibss.channel->band == IEEE80211_BAND_5GHZ ?
> > + IEEE80211_RATE_MANDATORY_A :
> > + IEEE80211_RATE_MANDATORY_B;
> > +
> > + for (j = 0; j < sband->n_bitrates; j++) {
> > + if (sband->bitrates[j].flags & flag)
> > + ibss.basic_rates |= j;
> > + }
> > + }
>
> couldn't we use ieee80211_mandatory_rates() here?

Yeah, but it would have to migrate to cfg80211.

johannes


2010-05-27 00:41:32

by Bruno Randolf

[permalink] [raw]
Subject: Re: [PATCHv2] mac80211: Fix basic rates for created IBSS networks

On Wednesday 26 May 2010 23:14:36 you wrote:
> Needs to be split up (setting bss_conf and the bss_change flag is a
> separate bugfix) and tested but this is what I think it should be like.

thanks for this!

> + } else {
> + /*
> + * If no rates were explicitly configured,
> + * use the mandatory rate set for 11b or
> + * 11a for maximum compatibility.
> + */
> + struct ieee80211_supported_band *sband =
> + wiphy->bands[ibss.channel->band];
> + int j;
> + u32 flag = ibss.channel->band == IEEE80211_BAND_5GHZ ?
> + IEEE80211_RATE_MANDATORY_A :
> + IEEE80211_RATE_MANDATORY_B;
> +
> + for (j = 0; j < sband->n_bitrates; j++) {
> + if (sband->bitrates[j].flags & flag)
> + ibss.basic_rates |= j;
> + }
> + }

couldn't we use ieee80211_mandatory_rates() here?

bruno

2010-05-27 08:11:19

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: mark 1, 2, 5.5 and 11Mbps as mandatory rates for 802.11b

On Thu, 2010-05-27 at 10:08 +0200, Johannes Berg wrote:
> On Thu, 2010-05-27 at 09:45 +0900, Bruno Randolf wrote:
> > IEEE802.11-2007 clause 18.2.3.3 (p640) states that 1, 2, 5.5 & 11 Mbits are
> > mandatory rates for what they call High Rate direct sequence spread spectrum
> > (HR/DSSS) PHY (with long PLCP).
>
> You're confused. Clause 18 is 11g, we knew that those were mandatory
> there, see the code you're modifying.
>
> Clause 15 is 11b, and it shouldn't change.

No wait, I'm confused. Clause 19 is 11g, and Clause 18 is 11b. But
what's Clause 15? Huh now I need to look back at this in more detail.

johannes


2010-05-26 14:14:41

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCHv2] mac80211: Fix basic rates for created IBSS networks

TBD

Needs to be split up (setting bss_conf and the bss_change flag is a
separate bugfix) and tested but this is what I think it should be like.

Signed-off-by: Johannes Berg <[email protected]>
---
include/net/cfg80211.h | 2 +
net/mac80211/ibss.c | 5 +++-
net/mac80211/ieee80211_i.h | 2 +
net/wireless/nl80211.c | 47 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 55 insertions(+), 1 deletion(-)

--- wireless-testing.orig/include/net/cfg80211.h 2010-05-26 15:43:20.000000000 +0200
+++ wireless-testing/include/net/cfg80211.h 2010-05-26 15:43:54.000000000 +0200
@@ -801,6 +801,7 @@ struct cfg80211_disassoc_request {
* @beacon_interval: beacon interval to use
* @privacy: this is a protected network, keys will be configured
* after joining
+ * @basic_rates: bitmap of basic rates to use when creating the IBSS
*/
struct cfg80211_ibss_params {
u8 *ssid;
@@ -809,6 +810,7 @@ struct cfg80211_ibss_params {
u8 *ie;
u8 ssid_len, ie_len;
u16 beacon_interval;
+ u32 basic_rates;
bool channel_fixed;
bool privacy;
};
--- wireless-testing.orig/net/wireless/nl80211.c 2010-05-26 15:46:01.000000000 +0200
+++ wireless-testing/net/wireless/nl80211.c 2010-05-26 16:08:28.000000000 +0200
@@ -3955,6 +3955,53 @@ static int nl80211_join_ibss(struct sk_b
}
}

+ if (info->attrs[NL80211_ATTR_BSS_BASIC_RATES]) {
+ u8 *rates = nla_data(info->attrs[NL80211_ATTR_BSS_BASIC_RATES]);
+ int n_rates = nla_len(info->attrs[NL80211_ATTR_BSS_BASIC_RATES]);
+ struct ieee80211_supported_band *sband =
+ wiphy->bands[ibss.channel->band];
+ int i, j;
+
+ if (n_rates == 0) {
+ err = -EINVAL;
+ goto out;
+ }
+
+ for (i = 0; i < n_rates; i++) {
+ int rate = (rates[i] & 0x7f) * 5;
+ bool found = false;
+
+ for (j = 0; j < sband->n_bitrates; j++) {
+ if (sband->bitrates[j].bitrate == rate) {
+ found = true;
+ ibss.basic_rates |= j;
+ break;
+ }
+ }
+ if (!found) {
+ err = -EINVAL;
+ goto out;
+ }
+ }
+ } else {
+ /*
+ * If no rates were explicitly configured,
+ * use the mandatory rate set for 11b or
+ * 11a for maximum compatibility.
+ */
+ struct ieee80211_supported_band *sband =
+ wiphy->bands[ibss.channel->band];
+ int j;
+ u32 flag = ibss.channel->band == IEEE80211_BAND_5GHZ ?
+ IEEE80211_RATE_MANDATORY_A :
+ IEEE80211_RATE_MANDATORY_B;
+
+ for (j = 0; j < sband->n_bitrates; j++) {
+ if (sband->bitrates[j].flags & flag)
+ ibss.basic_rates |= j;
+ }
+ }
+
err = cfg80211_join_ibss(rdev, dev, &ibss, connkeys);

out:
--- wireless-testing.orig/net/mac80211/ibss.c 2010-05-26 16:09:11.000000000 +0200
+++ wireless-testing/net/mac80211/ibss.c 2010-05-26 16:12:32.000000000 +0200
@@ -172,11 +172,13 @@ static void __ieee80211_sta_join_ibss(st
rcu_assign_pointer(ifibss->presp, skb);

sdata->vif.bss_conf.beacon_int = beacon_int;
+ sdata->vif.bss_conf.basic_rates = basic_rates;
bss_change = BSS_CHANGED_BEACON_INT;
bss_change |= ieee80211_reset_erp_info(sdata);
bss_change |= BSS_CHANGED_BSSID;
bss_change |= BSS_CHANGED_BEACON;
bss_change |= BSS_CHANGED_BEACON_ENABLED;
+ bss_change |= BSS_CHANGED_BASIC_RATES;
bss_change |= BSS_CHANGED_IBSS;
sdata->vif.bss_conf.ibss_joined = true;
ieee80211_bss_info_change_notify(sdata, bss_change);
@@ -529,7 +531,7 @@ static void ieee80211_sta_create_ibss(st
sdata->drop_unencrypted = 0;

__ieee80211_sta_join_ibss(sdata, bssid, sdata->vif.bss_conf.beacon_int,
- ifibss->channel, 3, /* first two are basic */
+ ifibss->channel, ifibss->basic_rates,
capability, 0);
}

@@ -910,6 +912,7 @@ int ieee80211_ibss_join(struct ieee80211
sdata->u.ibss.fixed_bssid = false;

sdata->u.ibss.privacy = params->privacy;
+ sdata->u.ibss.basic_rates = params->basic_rates;

sdata->vif.bss_conf.beacon_int = params->beacon_interval;

--- wireless-testing.orig/net/mac80211/ieee80211_i.h 2010-05-26 16:11:51.000000000 +0200
+++ wireless-testing/net/mac80211/ieee80211_i.h 2010-05-26 16:12:02.000000000 +0200
@@ -393,6 +393,8 @@ struct ieee80211_if_ibss {
unsigned long request;
unsigned long last_scan_completed;

+ u32 basic_rates;
+
bool timer_running;

bool fixed_bssid;



2010-05-27 12:09:24

by Benoit Papillault

[permalink] [raw]
Subject: Re: [PATCH] mac80211: mark 1, 2, 5.5 and 11Mbps as mandatory rates for 802.11b

Le 27/05/2010 10:08, Johannes Berg a écrit :
> On Thu, 2010-05-27 at 09:45 +0900, Bruno Randolf wrote:
>> IEEE802.11-2007 clause 18.2.3.3 (p640) states that 1, 2, 5.5& 11 Mbits are
>> mandatory rates for what they call High Rate direct sequence spread spectrum
>> (HR/DSSS) PHY (with long PLCP).
>
> You're confused. Clause 18 is 11g, we knew that those were mandatory
> there, see the code you're modifying.
>
> Clause 15 is 11b, and it shouldn't change.
>
> So NACK.
>
> johannes

I just dig through the various standards and besides there are still
some grey areas in my mind (like what is HR/DSSS/PBCC mentioned in
Clause 18 or DSSS-OFDM as specified in Clause 19, hopefully both are
optional), we have :

802.11a = Clause 17, called OFDM
802.11b = Clause 15 (called DSSS) & Clause 18 (called HR/DSSS).
802.11g = Clause 19, called ERP with lots of variants : ERP-DSSS/CCK,
ERP-OFDM, ERP-PBCC
802.11n = Clause 20, called HT for High Throughput, which also includes
DUP-OFDM

Clause 15 defines 1 Mbits and 2 Mbits rates, long PLCP only
Clause 18 defines 5.5 Mbits and 11 Mbits. Optionally, short PLCP is
available for 2 Mbits, 5.5 Mbits and 11 Mbits. Since it's optional, such
short PLCP rates are probably not part of the mandatory rates.

Basic rate set must includes mandatory rates set. This is apparently
what every other AP do today and I indeed see no points in adding more
rates to the basic rate set other than what is already defined in the
mandatory rate set (except we cannot rely on this rule when joining an
existing BSS/IBSS, of course).

Does the patch you posted is intended to create a 802.11b only or
802.11bg AP/IBSS with a 802.11g hardware for instance?

Regards,
Benoit

2010-05-27 00:45:43

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH] mac80211: mark 1, 2, 5.5 and 11Mbps as mandatory rates for 802.11b

IEEE802.11-2007 clause 18.2.3.3 (p640) states that 1, 2, 5.5 & 11 Mbits are
mandatory rates for what they call High Rate direct sequence spread spectrum
(HR/DSSS) PHY (with long PLCP).

Signed-off-by: Bruno Randolf <[email protected]>
---
net/wireless/util.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/wireless/util.c b/net/wireless/util.c
index 3416373..f6f4101 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -100,17 +100,17 @@ static void set_mandatory_flags_band(struct ieee80211_supported_band *sband,
case IEEE80211_BAND_2GHZ:
want = 7;
for (i = 0; i < sband->n_bitrates; i++) {
- if (sband->bitrates[i].bitrate == 10) {
+ if (sband->bitrates[i].bitrate == 10 ||
+ sband->bitrates[i].bitrate == 20 ||
+ sband->bitrates[i].bitrate == 55 ||
+ sband->bitrates[i].bitrate == 110) {
sband->bitrates[i].flags |=
IEEE80211_RATE_MANDATORY_B |
IEEE80211_RATE_MANDATORY_G;
want--;
}

- if (sband->bitrates[i].bitrate == 20 ||
- sband->bitrates[i].bitrate == 55 ||
- sband->bitrates[i].bitrate == 110 ||
- sband->bitrates[i].bitrate == 60 ||
+ if (sband->bitrates[i].bitrate == 60 ||
sband->bitrates[i].bitrate == 120 ||
sband->bitrates[i].bitrate == 240) {
sband->bitrates[i].flags |=
@@ -125,7 +125,7 @@ static void set_mandatory_flags_band(struct ieee80211_supported_band *sband,
sband->bitrates[i].flags |=
IEEE80211_RATE_ERP_G;
}
- WARN_ON(want != 0 && want != 3 && want != 6);
+ WARN_ON(want != 0 && want != 3);
break;
case IEEE80211_NUM_BANDS:
WARN_ON(1);


2010-05-26 13:12:26

by Juuso Oikarinen

[permalink] [raw]
Subject: Re: [PATCHv2] mac80211: Fix basic rates for created IBSS networks

On Wed, 2010-05-26 at 14:53 +0200, ext Johannes Berg wrote:
> On Wed, 2010-05-26 at 15:48 +0300, Juuso Oikarinen wrote:
> > Currently the mac80211 marks rates 1 and 2 mbps as basic rates for created
> > ad-hoc networks. To be certifiable, rates 1, 2, 5.5 and 11 need to be marked
> > basic.
> >
> > Change this.
> >
> > Signed-off-by: Juuso Oikarinen <[email protected]>
> > ---
> > net/mac80211/ibss.c | 3 ++-
> > 1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
> > index b2cc1fd..0b8360c 100644
> > --- a/net/mac80211/ibss.c
> > +++ b/net/mac80211/ibss.c
> > @@ -529,7 +529,8 @@ static void ieee80211_sta_create_ibss(struct ieee80211_sub_if_data *sdata)
> > sdata->drop_unencrypted = 0;
> >
> > __ieee80211_sta_join_ibss(sdata, bssid, sdata->vif.bss_conf.beacon_int,
> > - ifibss->channel, 3, /* first two are basic */
> > + ifibss->channel,
> > + 15, /* 1, 2, 5.5 and 11 are basic */
>
> It's still wrong for 5 GHz.

Yes, it is as wrong for 5GHz as it was before the change.

>
> We've had this discussion before. Making more rates required will break
> compatibility with 11b devices.

I don't understand this. AFAIK also in 11b all these rates are
mandatory. How come this breaks 11b?

This change is for creating IBSS, not joining. In case of joining, we
still adhere to the basic rates set by the creator, or that is what I
understand based on the code.

> What should be done here is add a basic rates parameter to the IBSS join
> nl80211 command so that you can decide at runtime which rates to use as
> basic.

Yeah, this is a solution needed latest when 5GHz support is implemented.

-Juuso

> johannes
>



2010-05-27 05:05:05

by Juuso Oikarinen

[permalink] [raw]
Subject: Re: [PATCHv2] mac80211: Fix basic rates for created IBSS networks

On Wed, 2010-05-26 at 16:14 +0200, ext Johannes Berg wrote:
> TBD
>
> Needs to be split up (setting bss_conf and the bss_change flag is a
> separate bugfix) and tested but this is what I think it should be like.

Yes, I think this would work. Are you planning on working on this patch?

-Juuso


> Signed-off-by: Johannes Berg <[email protected]>
> ---
> include/net/cfg80211.h | 2 +
> net/mac80211/ibss.c | 5 +++-
> net/mac80211/ieee80211_i.h | 2 +
> net/wireless/nl80211.c | 47 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 55 insertions(+), 1 deletion(-)
>
> --- wireless-testing.orig/include/net/cfg80211.h 2010-05-26 15:43:20.000000000 +0200
> +++ wireless-testing/include/net/cfg80211.h 2010-05-26 15:43:54.000000000 +0200
> @@ -801,6 +801,7 @@ struct cfg80211_disassoc_request {
> * @beacon_interval: beacon interval to use
> * @privacy: this is a protected network, keys will be configured
> * after joining
> + * @basic_rates: bitmap of basic rates to use when creating the IBSS
> */
> struct cfg80211_ibss_params {
> u8 *ssid;
> @@ -809,6 +810,7 @@ struct cfg80211_ibss_params {
> u8 *ie;
> u8 ssid_len, ie_len;
> u16 beacon_interval;
> + u32 basic_rates;
> bool channel_fixed;
> bool privacy;
> };
> --- wireless-testing.orig/net/wireless/nl80211.c 2010-05-26 15:46:01.000000000 +0200
> +++ wireless-testing/net/wireless/nl80211.c 2010-05-26 16:08:28.000000000 +0200
> @@ -3955,6 +3955,53 @@ static int nl80211_join_ibss(struct sk_b
> }
> }
>
> + if (info->attrs[NL80211_ATTR_BSS_BASIC_RATES]) {
> + u8 *rates = nla_data(info->attrs[NL80211_ATTR_BSS_BASIC_RATES]);
> + int n_rates = nla_len(info->attrs[NL80211_ATTR_BSS_BASIC_RATES]);
> + struct ieee80211_supported_band *sband =
> + wiphy->bands[ibss.channel->band];
> + int i, j;
> +
> + if (n_rates == 0) {
> + err = -EINVAL;
> + goto out;
> + }
> +
> + for (i = 0; i < n_rates; i++) {
> + int rate = (rates[i] & 0x7f) * 5;
> + bool found = false;
> +
> + for (j = 0; j < sband->n_bitrates; j++) {
> + if (sband->bitrates[j].bitrate == rate) {
> + found = true;
> + ibss.basic_rates |= j;
> + break;
> + }
> + }
> + if (!found) {
> + err = -EINVAL;
> + goto out;
> + }
> + }
> + } else {
> + /*
> + * If no rates were explicitly configured,
> + * use the mandatory rate set for 11b or
> + * 11a for maximum compatibility.
> + */
> + struct ieee80211_supported_band *sband =
> + wiphy->bands[ibss.channel->band];
> + int j;
> + u32 flag = ibss.channel->band == IEEE80211_BAND_5GHZ ?
> + IEEE80211_RATE_MANDATORY_A :
> + IEEE80211_RATE_MANDATORY_B;
> +
> + for (j = 0; j < sband->n_bitrates; j++) {
> + if (sband->bitrates[j].flags & flag)
> + ibss.basic_rates |= j;
> + }
> + }
> +
> err = cfg80211_join_ibss(rdev, dev, &ibss, connkeys);
>
> out:
> --- wireless-testing.orig/net/mac80211/ibss.c 2010-05-26 16:09:11.000000000 +0200
> +++ wireless-testing/net/mac80211/ibss.c 2010-05-26 16:12:32.000000000 +0200
> @@ -172,11 +172,13 @@ static void __ieee80211_sta_join_ibss(st
> rcu_assign_pointer(ifibss->presp, skb);
>
> sdata->vif.bss_conf.beacon_int = beacon_int;
> + sdata->vif.bss_conf.basic_rates = basic_rates;
> bss_change = BSS_CHANGED_BEACON_INT;
> bss_change |= ieee80211_reset_erp_info(sdata);
> bss_change |= BSS_CHANGED_BSSID;
> bss_change |= BSS_CHANGED_BEACON;
> bss_change |= BSS_CHANGED_BEACON_ENABLED;
> + bss_change |= BSS_CHANGED_BASIC_RATES;
> bss_change |= BSS_CHANGED_IBSS;
> sdata->vif.bss_conf.ibss_joined = true;
> ieee80211_bss_info_change_notify(sdata, bss_change);
> @@ -529,7 +531,7 @@ static void ieee80211_sta_create_ibss(st
> sdata->drop_unencrypted = 0;
>
> __ieee80211_sta_join_ibss(sdata, bssid, sdata->vif.bss_conf.beacon_int,
> - ifibss->channel, 3, /* first two are basic */
> + ifibss->channel, ifibss->basic_rates,
> capability, 0);
> }
>
> @@ -910,6 +912,7 @@ int ieee80211_ibss_join(struct ieee80211
> sdata->u.ibss.fixed_bssid = false;
>
> sdata->u.ibss.privacy = params->privacy;
> + sdata->u.ibss.basic_rates = params->basic_rates;
>
> sdata->vif.bss_conf.beacon_int = params->beacon_interval;
>
> --- wireless-testing.orig/net/mac80211/ieee80211_i.h 2010-05-26 16:11:51.000000000 +0200
> +++ wireless-testing/net/mac80211/ieee80211_i.h 2010-05-26 16:12:02.000000000 +0200
> @@ -393,6 +393,8 @@ struct ieee80211_if_ibss {
> unsigned long request;
> unsigned long last_scan_completed;
>
> + u32 basic_rates;
> +
> bool timer_running;
>
> bool fixed_bssid;
>
>



2010-05-27 04:48:35

by Juuso Oikarinen

[permalink] [raw]
Subject: Re: [PATCHv2] mac80211: Fix basic rates for created IBSS networks

On Wed, 2010-05-26 at 15:27 +0200, ext Johannes Berg wrote:
> On Wed, 2010-05-26 at 16:13 +0300, Juuso Oikarinen wrote:
>
> > > > diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
> > > > index b2cc1fd..0b8360c 100644
> > > > --- a/net/mac80211/ibss.c
> > > > +++ b/net/mac80211/ibss.c
> > > > @@ -529,7 +529,8 @@ static void ieee80211_sta_create_ibss(struct ieee80211_sub_if_data *sdata)
> > > > sdata->drop_unencrypted = 0;
> > > >
> > > > __ieee80211_sta_join_ibss(sdata, bssid, sdata->vif.bss_conf.beacon_int,
> > > > - ifibss->channel, 3, /* first two are basic */
> > > > + ifibss->channel,
> > > > + 15, /* 1, 2, 5.5 and 11 are basic */
> > >
> > > It's still wrong for 5 GHz.
> >
> > Yes, it is as wrong for 5GHz as it was before the change.
>
> No it wasn't wrong before the change, and only the comment is now
> wrong ...
>
> before the change, 6 and 9 mbps were basic for 5ghz, now it would be
> 5,9,12 and 18. Neither set actually overlaps with the mandatory rates,
> which is a bug I guess?

Ah, now I get it. So the rate bitmap changes - I had a confusion here.

> > > We've had this discussion before. Making more rates required will break
> > > compatibility with 11b devices.
> >
> > I don't understand this. AFAIK also in 11b all these rates are
> > mandatory. How come this breaks 11b?
>
> No, only 1mbit is really mandatory in 11b. So I guess our default should
> be just "1" rather than "3" or something like that.

That's weird. At least in certification PoV, AFAIK, a device would not
pass with just 1mbit even for 11b.

> > This change is for creating IBSS, not joining. In case of joining, we
> > still adhere to the basic rates set by the creator, or that is what I
> > understand based on the code.
> >
> > > What should be done here is add a basic rates parameter to the IBSS join
> > > nl80211 command so that you can decide at runtime which rates to use as
> > > basic.
> >
> > Yeah, this is a solution needed latest when 5GHz support is implemented.
>
> Well this hopefully doesn't come as a surprise to you, but mac80211 does
> support 5 GHz operation :)

Yeah, I know there is support (even the wl1271 has preliminary) but
there are also some issues ;)

So it seems to me that if we want this fixed, we would have to go with
an interface update?

> johannes
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



2010-05-27 08:27:28

by Juuso Oikarinen

[permalink] [raw]
Subject: Re: [PATCHv2] mac80211: Fix basic rates for created IBSS networks

On Thu, 2010-05-27 at 10:11 +0200, ext Johannes Berg wrote:
> On Thu, 2010-05-27 at 08:06 +0300, Juuso Oikarinen wrote:
> > On Wed, 2010-05-26 at 16:14 +0200, ext Johannes Berg wrote:
> > > TBD
> > >
> > > Needs to be split up (setting bss_conf and the bss_change flag is a
> > > separate bugfix) and tested but this is what I think it should be like.
> >
> > Yes, I think this would work. Are you planning on working on this patch?
>
> No.

Okay, I'll take the patch and work it further.

-Juuso

> johannes
>



2010-05-27 06:02:17

by Benoit Papillault

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH] mac80211: mark 1, 2, 5.5 and 11Mbps as mandatory rates for 802.11b

Le 27/05/2010 02:45, Bruno Randolf a ?crit :
> IEEE802.11-2007 clause 18.2.3.3 (p640) states that 1, 2, 5.5& 11 Mbits are
> mandatory rates for what they call High Rate direct sequence spread spectrum
> (HR/DSSS) PHY (with long PLCP).
>
> Signed-off-by: Bruno Randolf<[email protected]>
> ---
> net/wireless/util.c | 12 ++++++------
> 1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/net/wireless/util.c b/net/wireless/util.c
> index 3416373..f6f4101 100644
> --- a/net/wireless/util.c
> +++ b/net/wireless/util.c
> @@ -100,17 +100,17 @@ static void set_mandatory_flags_band(struct ieee80211_supported_band *sband,
> case IEEE80211_BAND_2GHZ:
> want = 7;
> for (i = 0; i< sband->n_bitrates; i++) {
> - if (sband->bitrates[i].bitrate == 10) {
> + if (sband->bitrates[i].bitrate == 10 ||
> + sband->bitrates[i].bitrate == 20 ||
> + sband->bitrates[i].bitrate == 55 ||
> + sband->bitrates[i].bitrate == 110) {
> sband->bitrates[i].flags |=
> IEEE80211_RATE_MANDATORY_B |
> IEEE80211_RATE_MANDATORY_G;
> want--;
> }
>
> - if (sband->bitrates[i].bitrate == 20 ||
> - sband->bitrates[i].bitrate == 55 ||
> - sband->bitrates[i].bitrate == 110 ||
> - sband->bitrates[i].bitrate == 60 ||
> + if (sband->bitrates[i].bitrate == 60 ||
> sband->bitrates[i].bitrate == 120 ||
> sband->bitrates[i].bitrate == 240) {
> sband->bitrates[i].flags |=
> @@ -125,7 +125,7 @@ static void set_mandatory_flags_band(struct ieee80211_supported_band *sband,
> sband->bitrates[i].flags |=
> IEEE80211_RATE_ERP_G;
> }
> - WARN_ON(want != 0&& want != 3&& want != 6);
> + WARN_ON(want != 0&& want != 3);
> break;
> case IEEE80211_NUM_BANDS:
> WARN_ON(1);
>

Acked-by: Benoit Papillault <[email protected]>

Regards,
Benoit