On Fri, Sep 16, 2011 at 15:59, Johannes Berg <johannes@xxxxxxxxxxxxxxxx> wrote:
> On Thu, 2011-09-15 at 13:25 +0300, Arik Nemtsov wrote:
>
>> + case NL80211_TDLS_ENABLE_LINK:
>> + rcu_read_lock();
>> + sta = sta_info_get(sdata, peer);
>> + if (sta) {
>> + set_sta_flags(sta, WLAN_STA_AUTHORIZED);
>> + sta->tdls_link_enabled = true;
>> + }
>> + rcu_read_unlock();
>> + break;
>
> This seems to require the station already having been added, but
> couldn't this create the data race you were worried about? Could you not
> simply just create the station once it is authorized?
The station is added only when the link is already authorized (see
wpa_tdls_enable_link() in wpa_s). This whole bit is kind of redundant.
A cleaner solution would be to just automatically authorize tdls
stations in ieee80211_add_station() and use WLAN_STA_TDLS_PEER instead
of tdls_link_enabled. I'll add this to my v2 list. Thanks.
While this creates a race, it's not the one i'm worried about. It will
also be solved automatically when the TDLS locking requirements are
met (as mentioned in a previous email).
But I agree your approach is cleaner.
>
>> + case NL80211_TDLS_DISABLE_LINK:
>> + rcu_read_lock();
>> + sta = sta_info_get(sdata, peer);
>> + if (sta) {
>> + sta->tdls_link_enabled = false;
>> + sta_info_destroy_addr(sdata, peer);
>> + }
>> + rcu_read_unlock();
>> + break;
>
> Isn't that equivalent to just deleting the station?
Yep. The whole sta->tdls_link_enabled thing is kind of redundant. v2.
Arik
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
