Re: Libertas: Association request to the driver failed

[Jonathan Cameron <jic23@cam.ac.uk>]

Just as a heads up, even with this patch I'm still getting
problems with association that didn't occur before. Just reverted
and all works fine.

I'll try and pin them down and report back later this afternoon.

Jonathan
> On Mon, Aug 10, 2009 at 12:37:00PM +0200, Roel Kluin wrote:
>> I think there was another problem in lbs_associate(),
>> the memcpy already affected rates->rates.
>>
>> Also in get_common_rates() I think we can safely move the
>> memset/memcpy, originally after label done, upwards.
>>
>> The patch below, if correct, is to be applied after the revert
> 
> I tested that and the driver still works fine for me. Thanks :)
> 
> Feel free to add my Tested-by: if you like.
> 
> Daniel
> 
> 
>> diff --git a/drivers/net/wireless/libertas/assoc.c b/drivers/net/wireless/libertas/assoc.c
>> index b9b3741..ba0164a 100644
>> --- a/drivers/net/wireless/libertas/assoc.c
>> +++ b/drivers/net/wireless/libertas/assoc.c
>> @@ -1,6 +1,7 @@
>>  /* Copyright (C) 2006, Red Hat, Inc. */
>>  
>>  #include <linux/types.h>
>> +#include <linux/kernel.h>
>>  #include <linux/etherdevice.h>
>>  #include <linux/ieee80211.h>
>>  #include <linux/if_arp.h>
>> @@ -43,41 +44,41 @@ static int get_common_rates(struct lbs_private *priv,
>>  	u16 *rates_size)
>>  {
>>  	u8 *card_rates = lbs_bg_rates;
>> -	size_t num_card_rates = sizeof(lbs_bg_rates);
>> -	int ret = 0, i, j;
>> -	u8 tmp[30];
>> +	int i, j;
>> +	u8 tmp[MAX_RATES * ARRAY_SIZE(lbs_bg_rates)];
>>  	size_t tmp_size = 0;
>>  
>>  	/* For each rate in card_rates that exists in rate1, copy to tmp */
>> -	for (i = 0; card_rates[i] && (i < num_card_rates); i++) {
>> -		for (j = 0; rates[j] && (j < *rates_size); j++) {
>> +	for (i = 0; i < ARRAY_SIZE(lbs_bg_rates) && card_rates[i]; i++) {
>> +		for (j = 0; j < *rates_size && rates[j]; j++) {
>>  			if (rates[j] == card_rates[i])
>>  				tmp[tmp_size++] = card_rates[i];
>>  		}
>>  	}
>>  
>>  	lbs_deb_hex(LBS_DEB_JOIN, "AP rates    ", rates, *rates_size);
>> -	lbs_deb_hex(LBS_DEB_JOIN, "card rates  ", card_rates, num_card_rates);
>> +	lbs_deb_hex(LBS_DEB_JOIN, "card rates  ", card_rates,
>> +			ARRAY_SIZE(lbs_bg_rates));
>>  	lbs_deb_hex(LBS_DEB_JOIN, "common rates", tmp, tmp_size);
>>  	lbs_deb_join("TX data rate 0x%02x\n", priv->cur_rate);
>>  
>> +	memset(rates, 0, *rates_size);
>> +	*rates_size = min_t(u16, tmp_size, *rates_size);
>> +	memcpy(rates, tmp, *rates_size);
>> +
>>  	if (!priv->enablehwauto) {
>>  		for (i = 0; i < tmp_size; i++) {
>>  			if (tmp[i] == priv->cur_rate)
>> -				goto done;
>> +				break;
>> +		}
>> +		if (i == tmp_size) {
>> +			lbs_pr_alert("Previously set fixed data rate %#x isn't "
>> +					"compatible with the network.\n",
>> +					priv->cur_rate);
>> +			return -1;
>>  		}
>> -		lbs_pr_alert("Previously set fixed data rate %#x isn't "
>> -		       "compatible with the network.\n", priv->cur_rate);
>> -		ret = -1;
>> -		goto done;
>>  	}
>> -	ret = 0;
>> -
>> -done:
>> -	memset(rates, 0, *rates_size);
>> -	*rates_size = min_t(int, tmp_size, *rates_size);
>> -	memcpy(rates, tmp, *rates_size);
>> -	return ret;
>> +	return 0;
>>  }
>>  
>>  
>> @@ -321,8 +322,8 @@ static int lbs_associate(struct lbs_private *priv,
>>  
>>  	rates = (struct mrvl_ie_rates_param_set *) pos;
>>  	rates->header.type = cpu_to_le16(TLV_TYPE_RATES);
>> -	memcpy(&rates->rates, &bss->rates, MAX_RATES);
>> -	tmplen = MAX_RATES;
>> +	tmplen = min_t(u16, ARRAY_SIZE(rates->rates), MAX_RATES);
>> +	memcpy(&rates->rates, &bss->rates, tmplen);
>>  	if (get_common_rates(priv, rates->rates, &tmplen)) {
>>  		ret = -1;
>>  		goto done;
>> @@ -598,7 +599,7 @@ static int lbs_adhoc_join(struct lbs_private *priv,
>>  
>>  	/* Copy Data rates from the rates recorded in scan response */
>>  	memset(cmd.bss.rates, 0, sizeof(cmd.bss.rates));
>> -	ratesize = min_t(u16, sizeof(cmd.bss.rates), MAX_RATES);
>> +	ratesize = min_t(u16, ARRAY_SIZE(cmd.bss.rates), MAX_RATES);
>>  	memcpy(cmd.bss.rates, bss->rates, ratesize);
>>  	if (get_common_rates(priv, cmd.bss.rates, &ratesize)) {
>>  		lbs_deb_join("ADHOC_JOIN: get_common_rates returned error.\n");
> 
> _______________________________________________
> libertas-dev mailing list
> libertas-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/libertas-dev
>