Re: [PATCH 1/2] cfg80211: initialize rate control after station inserted

[Johannes Berg <johannes@sipsolutions.net>]

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

Hi,

Thanks. I'll comment on both patches together. Can you please also tell
us what the problem is this is solving? I'm a bit lost.

I think both of these patches should just rolled into one since this is
also a mac80211 patch -- even if it's for the bit that interacts with
cfg80211.

However, I don't think I actually understand the changes.

>  	sta_apply_parameters(local, sta, params);
>  
> -	rate_control_rate_init(sta);
> -
>  	layer2_update = sdata->vif.type == NL80211_IFTYPE_AP_VLAN ||
>  		sdata->vif.type == NL80211_IFTYPE_AP;
>  
> @@ -742,13 +740,17 @@ static int ieee80211_add_station(struct wiphy *wiphy, struct net_device *dev,
>  		if (err == -EEXIST && layer2_update) {
>  			/* Need to update layer 2 devices on reassociation */
>  			sta = sta_info_get(local, mac);
> -			if (sta)
> +			if (sta) {
> +				rate_control_rate_init(sta);
>  				ieee80211_send_layer2_update(sta);
> +			}
>  		}

Why is this necessary? It should already have been called for this
station earlier?

>  		rcu_read_unlock();
>  		return err;
>  	}
>  
> +	rate_control_rate_init(sta);
> +

Also, I don't see anything between the old place that it was called and
the new place you're moving it to that could possibly change the station
parameters?

Same in ibss.c (not quoting it here) where you're only moving it to
after sta_info_insert() -- all that seems to do is add race conditions,
allowing other code to find not-yet-initialised stations.

So the only place I could see a change being necessary would be mlme.c,
but then only moving rate_control_rate_init() to after the flags
settings, not to after the insert.

Am I missing something?

johannes

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