Re: [PATCH 2/3] mac80211: Implement API to configure station specific rssi threshold

[Tamizh chelvam <tamizhr@codeaurora.org>]

On 2018-11-09 17:19, Johannes Berg wrote:
> On Mon, 2018-10-15 at 23:27 +0530, Tamizh chelvam wrote:
>> 
>> +		sta_mon_rssi_config_free(sta);
>> +		sta->rssi_hyst = rssi_hyst;
>> +		if (fixed_thold) {
>> +			if (n_rssi_tholds > 2) {
>> +				ret = -EINVAL;
>> +				goto out;
>> +			}
> 
> This might be slightly wrong, you free and then can still return an
> error.
> 
Sure. I'll move the free part after the validation check.

>> +			if (n_rssi_tholds == 1) {
>> +				sta->rssi_low = rssi_tholds[0];
>> +				sta->rssi_high = rssi_tholds[0];
>> +			} else {
>> +				sta->rssi_low = rssi_tholds[0];
>> +				sta->rssi_high = rssi_tholds[1];
>> +			}
>> +		} else {
>> +			const s32 *rssi_tholds;
>> +
>> +			rssi_tholds = kmemdup(rssi_tholds,
>> +					      n_rssi_tholds * sizeof(s32),
>> +					      GFP_KERNEL);
>> +			if (!rssi_tholds) {
>> +				ret = -ENOMEM;
>> +				goto out;
>> +			}
> 
> Similarly here, I guess you should do the allocation (and other error
> checking) before freeing.
> 
ditto, Sure. I'll move the free part after the validation check.

>> +			sta->rssi_tholds = rssi_tholds;
>> +			sta->n_rssi_tholds = n_rssi_tholds;
>> +			ieee80211_update_rssi_config(sta);
> 
> 
> 
>> +struct sta_mon_rssi_config {
>> +};
> 
> Huh?
> 
oops:( I have kept all configuring parameters in sta_info itself, 
mistakenly didn't remove this struct:(
> 
> The commit log also makes it sounds like mac80211 actually *supports*
> this, but clearly that's not the case. However you don't give any data
> to the driver either, did you lose a patch along the way? Previously 
> you
> had patch 5 ("mac80211: Implement functionality to monitor station's
> rssi cross event") and if I remember correctly I said you should squash
> some, but now that's not here?
> 

Thanks,
Tamizh.