[PATCH/RFC] mac80211: fix association with some APs

[PATCH/RFC] mac80211: fix association with some APs

[Helmut Schaa]

Some APs refuse association if the supported rates contained in the association
request do not match its own supported rates. This patch introduces a new
function which builds the intersection between the AP's supported rates and the
client's supported rates to work around such problems. The same approach is
already used in ipw2200 for example.

Signed-off-by: Helmut Schaa <hschaa@suse.de>

---
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index cb01995..8cc5805 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -650,6 +650,26 @@ static void ieee80211_authenticate(struct net_device *dev,
 	mod_timer(&ifsta->timer, jiffies + IEEE80211_AUTH_TIMEOUT);
 }
 
+static int ieee80211_compatible_rates(struct ieee80211_sta_bss *bss,
+				      struct ieee80211_supported_band *sband,
+				      u64 *rates)
+{
+	int i, j, count;
+	*rates = 0;
+	count = 0;
+	for (i = 0; i < bss->supp_rates_len; i++) {
+		int rate = (bss->supp_rates[i] & 0x7F) * 5;
+
+		for (j = 0; j < sband->n_bitrates; j++)
+			if (sband->bitrates[j].bitrate == rate) {
+				*rates |= BIT(j);
+				count++;
+				break;
+			}
+	}
+
+	return count;
+}
 
 static void ieee80211_send_assoc(struct net_device *dev,
 				 struct ieee80211_if_sta *ifsta)
@@ -658,11 +678,12 @@ static void ieee80211_send_assoc(struct net_device *dev,
 	struct sk_buff *skb;
 	struct ieee80211_mgmt *mgmt;
 	u8 *pos, *ies;
-	int i, len;
+	int i, len, count, rates_len, supp_rates_len;
 	u16 capab;
 	struct ieee80211_sta_bss *bss;
 	int wmm = 0;
 	struct ieee80211_supported_band *sband;
+	u64 rates = 0;
 
 	skb = dev_alloc_skb(local->hw.extra_tx_headroom +
 			    sizeof(*mgmt) + 200 + ifsta->extra_ie_len +
@@ -724,24 +745,37 @@ static void ieee80211_send_assoc(struct net_device *dev,
 	*pos++ = ifsta->ssid_len;
 	memcpy(pos, ifsta->ssid, ifsta->ssid_len);
 
+	rates_len = ieee80211_compatible_rates(bss, sband, &rates);
+	supp_rates_len = rates_len;
+	if (supp_rates_len > 8)
+		supp_rates_len = 8;
+
 	len = sband->n_bitrates;
-	if (len > 8)
-		len = 8;
-	pos = skb_put(skb, len + 2);
+	pos = skb_put(skb, supp_rates_len + 2);
 	*pos++ = WLAN_EID_SUPP_RATES;
-	*pos++ = len;
-	for (i = 0; i < len; i++) {
-		int rate = sband->bitrates[i].bitrate;
-		*pos++ = (u8) (rate / 5);
-	}
+	*pos++ = supp_rates_len;
 
-	if (sband->n_bitrates > len) {
-		pos = skb_put(skb, sband->n_bitrates - len + 2);
-		*pos++ = WLAN_EID_EXT_SUPP_RATES;
-		*pos++ = sband->n_bitrates - len;
-		for (i = len; i < sband->n_bitrates; i++) {
+	count = 0;
+	for (i = 0; i < sband->n_bitrates; i++) {
+		if (BIT(i) & rates) {
 			int rate = sband->bitrates[i].bitrate;
 			*pos++ = (u8) (rate / 5);
+			if (++count == 8)
+				break;
+		}
+	}
+
+	if (count == 8)
+	{
+		pos = skb_put(skb, rates_len - count + 2);
+		*pos++ = WLAN_EID_EXT_SUPP_RATES;
+		*pos++ = rates_len - count;
+
+		for (i++; i < sband->n_bitrates; i++) {
+			if (BIT(i) & rates) {
+				int rate = sband->bitrates[i].bitrate;
+				*pos++ = (u8) (rate / 5);
+			}
 		}
 	}
 

Re: [PATCH/RFC] mac80211: fix association with some APs

[Johannes Berg]

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

On Thu, 2008-05-08 at 10:30 +0200, Helmut Schaa wrote:
> Some APs refuse association if the supported rates contained in the association
> request do not match its own supported rates. This patch introduces a new
> function which builds the intersection between the AP's supported rates and the
> client's supported rates to work around such problems. The same approach is
> already used in ipw2200 for example.

Clearly, somebody didn't read the spec. *sigh*

Quote:

10.3.3.1.2, parameter table (emphasis mine):

OperationalRateSet

The set of data rates that the STA desires
to use for communication within the BSS.
The STA must be able to receive at each
of the data rates listed in the set. This set
                                     ^^^^^^^^
is a superset of the rates contained in the
^^^^^^^^^^^^^^^^^^^^^^^^^^
BSSBasicRateSet parameter.



>  
> +	rates_len = ieee80211_compatible_rates(bss, sband, &rates);
> +	supp_rates_len = rates_len;
> +	if (supp_rates_len > 8)
> +		supp_rates_len = 8;

Please add a comment here why we do this, preferably including at least
one broken AP (so we can blame somebody ;) )

> +	if (count == 8)
> +	{

and fix this tiny coding style problem please.

johannes

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

Re: [PATCH/RFC v2] mac80211: fix association with some APs

[Helmut Schaa]

Some APs refuse association if the supported rates contained in the
association request do not match its own supported rates. This patch
introduces a new function which builds the intersection between the AP's
supported rates and the client's supported rates to work around such
problems. The same approach is already used in ipw2200 for example.

Signed-off-by: Helmut Schaa <hschaa@suse.de>

---
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index cb01995..91d96d9 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -650,6 +650,26 @@ static void ieee80211_authenticate(struct net_device *dev,
 	mod_timer(&ifsta->timer, jiffies + IEEE80211_AUTH_TIMEOUT);
 }
 
+static int ieee80211_compatible_rates(struct ieee80211_sta_bss *bss,
+				      struct ieee80211_supported_band *sband,
+				      u64 *rates)
+{
+	int i, j, count;
+	*rates = 0;
+	count = 0;
+	for (i = 0; i < bss->supp_rates_len; i++) {
+		int rate = (bss->supp_rates[i] & 0x7F) * 5;
+
+		for (j = 0; j < sband->n_bitrates; j++)
+			if (sband->bitrates[j].bitrate == rate) {
+				*rates |= BIT(j);
+				count++;
+				break;
+			}
+	}
+
+	return count;
+}
 
 static void ieee80211_send_assoc(struct net_device *dev,
 				 struct ieee80211_if_sta *ifsta)
@@ -658,11 +678,12 @@ static void ieee80211_send_assoc(struct net_device *dev,
 	struct sk_buff *skb;
 	struct ieee80211_mgmt *mgmt;
 	u8 *pos, *ies;
-	int i, len;
+	int i, len, count, rates_len, supp_rates_len;
 	u16 capab;
 	struct ieee80211_sta_bss *bss;
 	int wmm = 0;
 	struct ieee80211_supported_band *sband;
+	u64 rates = 0;
 
 	skb = dev_alloc_skb(local->hw.extra_tx_headroom +
 			    sizeof(*mgmt) + 200 + ifsta->extra_ie_len +
@@ -724,24 +745,39 @@ static void ieee80211_send_assoc(struct net_device *dev,
 	*pos++ = ifsta->ssid_len;
 	memcpy(pos, ifsta->ssid, ifsta->ssid_len);
 
+	/* all supported rates should be added here but some APs
+	 * (e.g. D-Link DAP 1353 in b-only mode) don't like that
+	 * Therefore only add rates the AP supports */
+	rates_len = ieee80211_compatible_rates(bss, sband, &rates);
+	supp_rates_len = rates_len;
+	if (supp_rates_len > 8)
+		supp_rates_len = 8;
+
 	len = sband->n_bitrates;
-	if (len > 8)
-		len = 8;
-	pos = skb_put(skb, len + 2);
+	pos = skb_put(skb, supp_rates_len + 2);
 	*pos++ = WLAN_EID_SUPP_RATES;
-	*pos++ = len;
-	for (i = 0; i < len; i++) {
-		int rate = sband->bitrates[i].bitrate;
-		*pos++ = (u8) (rate / 5);
-	}
+	*pos++ = supp_rates_len;
 
-	if (sband->n_bitrates > len) {
-		pos = skb_put(skb, sband->n_bitrates - len + 2);
-		*pos++ = WLAN_EID_EXT_SUPP_RATES;
-		*pos++ = sband->n_bitrates - len;
-		for (i = len; i < sband->n_bitrates; i++) {
+	count = 0;
+	for (i = 0; i < sband->n_bitrates; i++) {
+		if (BIT(i) & rates) {
 			int rate = sband->bitrates[i].bitrate;
 			*pos++ = (u8) (rate / 5);
+			if (++count == 8)
+				break;
+		}
+	}
+
+	if (count == 8) {
+		pos = skb_put(skb, rates_len - count + 2);
+		*pos++ = WLAN_EID_EXT_SUPP_RATES;
+		*pos++ = rates_len - count;
+
+		for (i++; i < sband->n_bitrates; i++) {
+			if (BIT(i) & rates) {
+				int rate = sband->bitrates[i].bitrate;
+				*pos++ = (u8) (rate / 5);
+			}
 		}
 	}
 

Re: [PATCH/RFC v2] mac80211: fix association with some APs

[Johannes Berg]

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

On Thu, 2008-05-08 at 13:34 +0200, Helmut Schaa wrote:
> Some APs refuse association if the supported rates contained in the
> association request do not match its own supported rates. This patch
> introduces a new function which builds the intersection between the AP's
> supported rates and the client's supported rates to work around such
> problems. The same approach is already used in ipw2200 for example.
> 
> Signed-off-by: Helmut Schaa <hschaa@suse.de>

Thanks. Looks ok to me.

> ---
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index cb01995..91d96d9 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -650,6 +650,26 @@ static void ieee80211_authenticate(struct net_device *dev,
>  	mod_timer(&ifsta->timer, jiffies + IEEE80211_AUTH_TIMEOUT);
>  }
>  
> +static int ieee80211_compatible_rates(struct ieee80211_sta_bss *bss,
> +				      struct ieee80211_supported_band *sband,
> +				      u64 *rates)
> +{
> +	int i, j, count;
> +	*rates = 0;
> +	count = 0;
> +	for (i = 0; i < bss->supp_rates_len; i++) {
> +		int rate = (bss->supp_rates[i] & 0x7F) * 5;
> +
> +		for (j = 0; j < sband->n_bitrates; j++)
> +			if (sband->bitrates[j].bitrate == rate) {
> +				*rates |= BIT(j);
> +				count++;
> +				break;
> +			}
> +	}
> +
> +	return count;
> +}
>  
>  static void ieee80211_send_assoc(struct net_device *dev,
>  				 struct ieee80211_if_sta *ifsta)
> @@ -658,11 +678,12 @@ static void ieee80211_send_assoc(struct net_device *dev,
>  	struct sk_buff *skb;
>  	struct ieee80211_mgmt *mgmt;
>  	u8 *pos, *ies;
> -	int i, len;
> +	int i, len, count, rates_len, supp_rates_len;
>  	u16 capab;
>  	struct ieee80211_sta_bss *bss;
>  	int wmm = 0;
>  	struct ieee80211_supported_band *sband;
> +	u64 rates = 0;
>  
>  	skb = dev_alloc_skb(local->hw.extra_tx_headroom +
>  			    sizeof(*mgmt) + 200 + ifsta->extra_ie_len +
> @@ -724,24 +745,39 @@ static void ieee80211_send_assoc(struct net_device *dev,
>  	*pos++ = ifsta->ssid_len;
>  	memcpy(pos, ifsta->ssid, ifsta->ssid_len);
>  
> +	/* all supported rates should be added here but some APs
> +	 * (e.g. D-Link DAP 1353 in b-only mode) don't like that
> +	 * Therefore only add rates the AP supports */
> +	rates_len = ieee80211_compatible_rates(bss, sband, &rates);
> +	supp_rates_len = rates_len;
> +	if (supp_rates_len > 8)
> +		supp_rates_len = 8;
> +
>  	len = sband->n_bitrates;
> -	if (len > 8)
> -		len = 8;
> -	pos = skb_put(skb, len + 2);
> +	pos = skb_put(skb, supp_rates_len + 2);
>  	*pos++ = WLAN_EID_SUPP_RATES;
> -	*pos++ = len;
> -	for (i = 0; i < len; i++) {
> -		int rate = sband->bitrates[i].bitrate;
> -		*pos++ = (u8) (rate / 5);
> -	}
> +	*pos++ = supp_rates_len;
>  
> -	if (sband->n_bitrates > len) {
> -		pos = skb_put(skb, sband->n_bitrates - len + 2);
> -		*pos++ = WLAN_EID_EXT_SUPP_RATES;
> -		*pos++ = sband->n_bitrates - len;
> -		for (i = len; i < sband->n_bitrates; i++) {
> +	count = 0;
> +	for (i = 0; i < sband->n_bitrates; i++) {
> +		if (BIT(i) & rates) {
>  			int rate = sband->bitrates[i].bitrate;
>  			*pos++ = (u8) (rate / 5);
> +			if (++count == 8)
> +				break;
> +		}
> +	}
> +
> +	if (count == 8) {
> +		pos = skb_put(skb, rates_len - count + 2);
> +		*pos++ = WLAN_EID_EXT_SUPP_RATES;
> +		*pos++ = rates_len - count;
> +
> +		for (i++; i < sband->n_bitrates; i++) {
> +			if (BIT(i) & rates) {
> +				int rate = sband->bitrates[i].bitrate;
> +				*pos++ = (u8) (rate / 5);
> +			}
>  		}
>  	}
>  
> 

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

Re: [PATCH/RFC v2] mac80211: fix association with some APs

[Tomas Winkler]

On Thu, May 8, 2008 at 2:36 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Thu, 2008-05-08 at 13:34 +0200, Helmut Schaa wrote:
>  > Some APs refuse association if the supported rates contained in the
>  > association request do not match its own supported rates. This patch
>  > introduces a new function which builds the intersection between the AP's
>  > supported rates and the client's supported rates to work around such
>  > problems. The same approach is already used in ipw2200 for example.
>  >
>  > Signed-off-by: Helmut Schaa <hschaa@suse.de>
>
>  Thanks. Looks ok to me.
>
>
>
Are we sure there is no  firmware with the fix for this AP?

Thanks
Tomas

Re: [PATCH/RFC v2] mac80211: fix association with some APs

[Johannes Berg]

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


> Are we sure there is no  firmware with the fix for this AP?

Even if there is, does it matter? Will everybody install it? It's not
like we're doing something that could cause problems since we announce
the subset that both we and the AP can support.

johannes

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

Re: [PATCH/RFC v2] mac80211: fix association with some APs

[Tomas Winkler]

On Fri, May 9, 2008 at 12:51 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
>
>  > Are we sure there is no  firmware with the fix for this AP?
>
>  Even if there is, does it matter? Will everybody install it? It's not
>  like we're doing something that could cause problems since we announce
>  the subset that both we and the AP can support.

Fair enough.
Tomas

Re: [PATCH/RFC v2] mac80211: fix association with some APs

[Kalle Valo]

Tomas Winkler <tomasw@gmail.com> writes:

> On Thu, May 8, 2008 at 2:36 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
>> On Thu, 2008-05-08 at 13:34 +0200, Helmut Schaa wrote:
>>  > Some APs refuse association if the supported rates contained in the
>>  > association request do not match its own supported rates. This patch
>>  > introduces a new function which builds the intersection between the AP's
>>  > supported rates and the client's supported rates to work around such
>>  > problems. The same approach is already used in ipw2200 for example.
>>  >
>>  > Signed-off-by: Helmut Schaa <hschaa@suse.de>
>>
>>  Thanks. Looks ok to me.
>
> Are we sure there is no  firmware with the fix for this AP?

Most probably this isn't the only AP which has a similar bug, so it's
better to add a workaround anyway. Let's just hope that it doesn't
create any regressions.

-- 
Kalle Valo

Re: [PATCH/RFC v2] mac80211: fix association with some APs

[Johannes Berg]

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


> Most probably this isn't the only AP which has a similar bug, so it's
> better to add a workaround anyway. Let's just hope that it doesn't
> create any regressions.

Speaking of which, what about the Nokia AP bug that cx3110x works
around? :)

johannes

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

Re: [PATCH/RFC v2] mac80211: fix association with some APs

[Kalle Valo]

Johannes Berg <johannes@sipsolutions.net> writes:

>> Most probably this isn't the only AP which has a similar bug, so it's
>> better to add a workaround anyway. Let's just hope that it doesn't
>> create any regressions.
>
> Speaking of which, what about the Nokia AP bug that cx3110x works
> around? :)

Hah hah, I had forgotten that one altogether :) I don't think we want
to take that route, that is enabling workarounds for APs based on
BSSID. That's going to be a maintainance nightmare in the long run, at
least in my opinion.

We should have one A032 in our office somewhere, I'll try to find it
and see how b43 works with it.

(For others wondering what we are talking about: Nokia 770/N800/N810
products enable long slot time when associating to Nokia A032 AP to
workaround a bug in the AP.)

-- 
Kalle Valo