[PATCH for 2.6.28] mac80211: fix basic rates

[PATCH for 2.6.28] mac80211: fix basic rates

[Johannes Berg]

This fixes the basic rate bitmap calculation in mac80211.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Cc: stable@kernel.org [2.6.25, 2.6.26, 2.6.27]
---
Is this appropriate for stable? It's a trivial and obvious fix, and can
throw off the duration calculation, but it's likely nobody will ever
really notice...

 net/mac80211/mlme.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

--- everything.orig/net/mac80211/mlme.c	2008-10-29 18:08:12.000000000 +0100
+++ everything/net/mac80211/mlme.c	2008-10-29 18:08:19.000000000 +0100
@@ -1307,8 +1307,9 @@ static void ieee80211_rx_mgmt_assoc_resp
 			have_higher_than_11mbit = true;
 
 		for (j = 0; j < sband->n_bitrates; j++) {
-			if (sband->bitrates[j].bitrate == rate)
-				rates |= BIT(j);
+			if (sband->bitrates[j].bitrate != rate)
+				continue;
+			rates |= BIT(j);
 			if (elems.supp_rates[i] & 0x80)
 				basic_rates |= BIT(j);
 		}
@@ -1321,8 +1322,9 @@ static void ieee80211_rx_mgmt_assoc_resp
 			have_higher_than_11mbit = true;
 
 		for (j = 0; j < sband->n_bitrates; j++) {
-			if (sband->bitrates[j].bitrate == rate)
-				rates |= BIT(j);
+			if (sband->bitrates[j].bitrate != rate)
+				continue;
+			rates |= BIT(j);
 			if (elems.ext_supp_rates[i] & 0x80)
 				basic_rates |= BIT(j);
 		}

Re: [PATCH for 2.6.28] mac80211: fix basic rates

[Tomas Winkler]

On Wed, Oct 29, 2008 at 7:14 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> This fixes the basic rate bitmap calculation in mac80211.
>
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> Cc: stable@kernel.org [2.6.25, 2.6.26, 2.6.27]
> ---
> Is this appropriate for stable? It's a trivial and obvious fix, and can
> throw off the duration calculation, but it's likely nobody will ever
> really notice...

You would should fail association if one of the basic rates is not
supported  by the station
Tomas

>
>  net/mac80211/mlme.c |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> --- everything.orig/net/mac80211/mlme.c 2008-10-29 18:08:12.000000000 +0100
> +++ everything/net/mac80211/mlme.c      2008-10-29 18:08:19.000000000 +0100
> @@ -1307,8 +1307,9 @@ static void ieee80211_rx_mgmt_assoc_resp
>                        have_higher_than_11mbit = true;
>
>                for (j = 0; j < sband->n_bitrates; j++) {
> -                       if (sband->bitrates[j].bitrate == rate)
> -                               rates |= BIT(j);
> +                       if (sband->bitrates[j].bitrate != rate)
> +                               continue;
> +                       rates |= BIT(j);
>                        if (elems.supp_rates[i] & 0x80)
>                                basic_rates |= BIT(j);
>                }
> @@ -1321,8 +1322,9 @@ static void ieee80211_rx_mgmt_assoc_resp
>                        have_higher_than_11mbit = true;
>
>                for (j = 0; j < sband->n_bitrates; j++) {
> -                       if (sband->bitrates[j].bitrate == rate)
> -                               rates |= BIT(j);
> +                       if (sband->bitrates[j].bitrate != rate)
> +                               continue;
> +                       rates |= BIT(j);
>                        if (elems.ext_supp_rates[i] & 0x80)
>                                basic_rates |= BIT(j);
>                }
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH for 2.6.28] mac80211: fix basic rates

[Johannes Berg]

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

On Thu, 2008-10-30 at 15:23 +0200, Tomas Winkler wrote:
> On Wed, Oct 29, 2008 at 7:14 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > This fixes the basic rate bitmap calculation in mac80211.
> >
> > Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> > Cc: stable@kernel.org [2.6.25, 2.6.26, 2.6.27]
> > ---
> > Is this appropriate for stable? It's a trivial and obvious fix, and can
> > throw off the duration calculation, but it's likely nobody will ever
> > really notice...
> 
> You would should fail association if one of the basic rates is not
> supported  by the station

No, this has nothing to do with association, it's just setting the
internal variable about which rates are basic in the BSS.

johannes

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

Re: [PATCH for 2.6.28] mac80211: fix basic rates

[Tomas Winkler]

On Thu, Oct 30, 2008 at 4:24 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Thu, 2008-10-30 at 15:23 +0200, Tomas Winkler wrote:
>> On Wed, Oct 29, 2008 at 7:14 PM, Johannes Berg
>> <johannes@sipsolutions.net> wrote:
>> > This fixes the basic rate bitmap calculation in mac80211.
>> >
>> > Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
>> > Cc: stable@kernel.org [2.6.25, 2.6.26, 2.6.27]
>> > ---
>> > Is this appropriate for stable? It's a trivial and obvious fix, and can
>> > throw off the duration calculation, but it's likely nobody will ever
>> > really notice...
>>
>> You would should fail association if one of the basic rates is not
>> supported  by the station
>
> No, this has nothing to do with association, it's just setting the
> internal variable about which rates are basic in the BSS.

I understand that but What your patch is doing it is preventing
of setting basic rates for ratest that are not sported by the driver,
but according
the spec it is not permissible that driver will not support one of the
basic rates.

Tomas

Re: [PATCH for 2.6.28] mac80211: fix basic rates

[Johannes Berg]

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

On Thu, 2008-10-30 at 16:39 +0200, Tomas Winkler wrote:
> On Thu, Oct 30, 2008 at 4:24 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > On Thu, 2008-10-30 at 15:23 +0200, Tomas Winkler wrote:
> >> On Wed, Oct 29, 2008 at 7:14 PM, Johannes Berg
> >> <johannes@sipsolutions.net> wrote:
> >> > This fixes the basic rate bitmap calculation in mac80211.
> >> >
> >> > Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> >> > Cc: stable@kernel.org [2.6.25, 2.6.26, 2.6.27]
> >> > ---
> >> > Is this appropriate for stable? It's a trivial and obvious fix, and can
> >> > throw off the duration calculation, but it's likely nobody will ever
> >> > really notice...
> >>
> >> You would should fail association if one of the basic rates is not
> >> supported  by the station
> >
> > No, this has nothing to do with association, it's just setting the
> > internal variable about which rates are basic in the BSS.
> 
> I understand that but What your patch is doing it is preventing
> of setting basic rates for ratest that are not sported by the driver,
> but according
> the spec it is not permissible that driver will not support one of the
> basic rates.

I don't understand. My patch is simply fixing the basic rate bitmap
calculation. If we don't support all basic rates then the AP shall have
denied the association, or we probably didn't even ask it.

The current code there is just wrong, it will always set _all_ rates in
the basic rate bitmap rather than just those that really are basic
rates.

johannes

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

Re: [PATCH for 2.6.28] mac80211: fix basic rates

[Tomas Winkler]

On Thu, Oct 30, 2008 at 4:41 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Thu, 2008-10-30 at 16:39 +0200, Tomas Winkler wrote:
>> On Thu, Oct 30, 2008 at 4:24 PM, Johannes Berg
>> <johannes@sipsolutions.net> wrote:
>> > On Thu, 2008-10-30 at 15:23 +0200, Tomas Winkler wrote:
>> >> On Wed, Oct 29, 2008 at 7:14 PM, Johannes Berg
>> >> <johannes@sipsolutions.net> wrote:
>> >> > This fixes the basic rate bitmap calculation in mac80211.
>> >> >
>> >> > Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
>> >> > Cc: stable@kernel.org [2.6.25, 2.6.26, 2.6.27]
>> >> > ---
>> >> > Is this appropriate for stable? It's a trivial and obvious fix, and can
>> >> > throw off the duration calculation, but it's likely nobody will ever
>> >> > really notice...
>> >>
>> >> You would should fail association if one of the basic rates is not
>> >> supported  by the station
>> >
>> > No, this has nothing to do with association, it's just setting the
>> > internal variable about which rates are basic in the BSS.
>>
>> I understand that but What your patch is doing it is preventing
>> of setting basic rates for ratest that are not sported by the driver,
>> but according
>> the spec it is not permissible that driver will not support one of the
>> basic rates.
>
> I don't understand. My patch is simply fixing the basic rate bitmap
> calculation. If we don't support all basic rates then the AP shall have
> denied the association, or we probably didn't even ask it.
>
> The current code there is just wrong, it will always set _all_ rates in
> the basic rate bitmap rather than just those that really are basic
> rates.

Oh, I now I got it out of index. What about coding it in more readable way?


for (i = 0; i < elems.supp_rates_len; i++) {
                int rate = (elems.supp_rates[i] & 0x7f) * 5;
                bool is_basic = !!(elems.supp_rates[i] & 0x80);

                if (rate > 110)
                        have_higher_than_11mbit = true;

                for (j = 0; j < sband->n_bitrates; j++) {
                        if (sband->bitrates[j].bitrate == rate) {
                                rates |= BIT(j);
                                if (is_basic)
                                        basic_rates |= BIT(j);
                                break;
                        }
                }
        }

Tomas

Re: [PATCH for 2.6.28] mac80211: fix basic rates

[Johannes Berg]

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

On Thu, 2008-10-30 at 16:51 +0200, Tomas Winkler wrote:

> Oh, I now I got it out of index. What about coding it in more readable way?
> 
> 
> for (i = 0; i < elems.supp_rates_len; i++) {
>                 int rate = (elems.supp_rates[i] & 0x7f) * 5;
>                 bool is_basic = !!(elems.supp_rates[i] & 0x80);
> 
>                 if (rate > 110)
>                         have_higher_than_11mbit = true;
> 
>                 for (j = 0; j < sband->n_bitrates; j++) {
>                         if (sband->bitrates[j].bitrate == rate) {
>                                 rates |= BIT(j);
>                                 if (is_basic)
>                                         basic_rates |= BIT(j);
>                                 break;

yeah, that works too, and the break is probably a good idea, want to
send a patch to supersede this one?

johannes

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