[PATCH] mac80211: use min rate as basic rate for buggy APs

[PATCH] mac80211: use min rate as basic rate for buggy APs

[Eliad Peller]

Some buggy APs (and even P2P_GO) don't advertise their
basic rates in the association response.

In such case, use the min supported rate as the
basic rate.

Reported-by: Pontus Fuchs <pontus.fuchs@gmail.com>
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 net/mac80211/mlme.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 0e5d8da..064df37 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1482,6 +1482,7 @@ static bool ieee80211_assoc_success(struct ieee80211_work *wk,
 	int i, j, err;
 	bool have_higher_than_11mbit = false;
 	u16 ap_ht_cap_flags;
+	int min_rate = INT_MAX, min_rate_index = -1;
 
 	/* AssocResp and ReassocResp have identical structure */
 
@@ -1537,6 +1538,10 @@ static bool ieee80211_assoc_success(struct ieee80211_work *wk,
 				rates |= BIT(j);
 				if (is_basic)
 					basic_rates |= BIT(j);
+				if (rate < min_rate) {
+					min_rate = rate;
+					min_rate_index = j;
+				}
 				break;
 			}
 		}
@@ -1554,11 +1559,22 @@ static bool ieee80211_assoc_success(struct ieee80211_work *wk,
 				rates |= BIT(j);
 				if (is_basic)
 					basic_rates |= BIT(j);
+				if (rate < min_rate) {
+					min_rate = rate;
+					min_rate_index = j;
+				}
 				break;
 			}
 		}
 	}
 
+	/*
+	 * some buggy APs don't advertise basic_rates. use the lowest
+	 * supported rate instead.
+	 */
+	if (unlikely(!basic_rates) && min_rate_index >= 0)
+		basic_rates = BIT(min_rate_index);
+
 	sta->sta.supp_rates[wk->chan->band] = rates;
 	sdata->vif.bss_conf.basic_rates = basic_rates;
 
-- 
1.7.6.401.g6a319

Re: [PATCH] mac80211: use min rate as basic rate for buggy APs

[Stanislaw Gruszka]

On Tue, Oct 25, 2011 at 10:26:34AM +0200, Eliad Peller wrote:
> Some buggy APs (and even P2P_GO) don't advertise their
> basic rates in the association response.
> 
> In such case, use the min supported rate as the
> basic rate.

Can we do this in simpler way?

   if (unlikely(!basic_rates))
	basic_rates = BIT(ffs(rates) - 1);

Stanislaw

> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---
>  net/mac80211/mlme.c |   16 ++++++++++++++++
>  1 files changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index 0e5d8da..064df37 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -1482,6 +1482,7 @@ static bool ieee80211_assoc_success(struct ieee80211_work *wk,
>  	int i, j, err;
>  	bool have_higher_than_11mbit = false;
>  	u16 ap_ht_cap_flags;
> +	int min_rate = INT_MAX, min_rate_index = -1;
>  
>  	/* AssocResp and ReassocResp have identical structure */
>  
> @@ -1537,6 +1538,10 @@ static bool ieee80211_assoc_success(struct ieee80211_work *wk,
>  				rates |= BIT(j);
>  				if (is_basic)
>  					basic_rates |= BIT(j);
> +				if (rate < min_rate) {
> +					min_rate = rate;
> +					min_rate_index = j;
> +				}
>  				break;
>  			}
>  		}
> @@ -1554,11 +1559,22 @@ static bool ieee80211_assoc_success(struct ieee80211_work *wk,
>  				rates |= BIT(j);
>  				if (is_basic)
>  					basic_rates |= BIT(j);
> +				if (rate < min_rate) {
> +					min_rate = rate;
> +					min_rate_index = j;
> +				}
>  				break;
>  			}
>  		}
>  	}
>  
> +	/*
> +	 * some buggy APs don't advertise basic_rates. use the lowest
> +	 * supported rate instead.
> +	 */
> +	if (unlikely(!basic_rates) && min_rate_index >= 0)
> +		basic_rates = BIT(min_rate_index);
> +
>  	sta->sta.supp_rates[wk->chan->band] = rates;
>  	sdata->vif.bss_conf.basic_rates = basic_rates;
>  
> -- 
> 1.7.6.401.g6a319
> 
> --
> 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] mac80211: use min rate as basic rate for buggy APs

[Eliad Peller]

On Tue, Oct 25, 2011 at 3:56 PM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> On Tue, Oct 25, 2011 at 10:26:34AM +0200, Eliad Peller wrote:
>> Some buggy APs (and even P2P_GO) don't advertise their
>> basic rates in the association response.
>>
>> In such case, use the min supported rate as the
>> basic rate.
>
> Can we do this in simpler way?
>
>   if (unlikely(!basic_rates))
>        basic_rates = BIT(ffs(rates) - 1);
>
the bitrates array is not sorted, so i guess not (unless we want to
choose an arbitrary rate).

Eliad.

Re: [PATCH] mac80211: use min rate as basic rate for buggy APs

[Johannes Berg]

On 10/25/2011 10:26 AM, Eliad Peller wrote:
> Some buggy APs (and even P2P_GO) don't advertise their
> basic rates in the association response.
>
> In such case, use the min supported rate as the
> basic rate.

I seem to vaguely remember discussing using mandatory rates, did we?

Also this seems like a last resort fallback, I'd really also try using 
the probe response information -- we should have that information still 
at this point. But maybe that doesn't make sense -- fix the APs instead!

I'd really like a (debug?) message here though. This is a stupid AP, and 
I will need to know if I'm debugging some throughput issues etc.

johannes

Re: [PATCH] mac80211: use min rate as basic rate for buggy APs

[Eliad Peller]

On Wed, Oct 26, 2011 at 11:53 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On 10/25/2011 10:26 AM, Eliad Peller wrote:
>>
>> Some buggy APs (and even P2P_GO) don't advertise their
>> basic rates in the association response.
>>
>> In such case, use the min supported rate as the
>> basic rate.
>
> I seem to vaguely remember discussing using mandatory rates, did we?
>
yes.
the problem with the mandatory rates is that we shouldn't use CCK
rates when working as p2p_cli (there *shouldn't* be such GO, but this
was an actual case...)

> Also this seems like a last resort fallback, I'd really also try using the
> probe response information -- we should have that information still at this
> point. But maybe that doesn't make sense -- fix the APs instead!
>
right.

> I'd really like a (debug?) message here though. This is a stupid AP, and I
> will need to know if I'm debugging some throughput issues etc.
sure. i'll add it.

thanks for the review.
Eliad.