Search Linux Wireless

Re: [RFC 2/2] mac80211: Optimize sta lookup for many VIFs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2013-03-15 at 14:13 -0700, greearb@xxxxxxxxxxxxxxx wrote:
 
> --- a/net/mac80211/ieee80211_i.h
> +++ b/net/mac80211/ieee80211_i.h
> @@ -669,6 +669,12 @@ struct ieee80211_sub_if_data {
>  	/* count for keys needing tailroom space allocation */
>  	int crypto_tx_tailroom_needed_cnt;
>  
> +	/* A pointer to some station associated with this interface, or
> +	 * NULL.  This aids oportunistic lookup for sta_info objects when

typo: opportunistic. Also it should probably say "allows opportunistic
lookup" :-)

> +	/* Shortcut for finding station entries when sdata is a station */
> +	some_sta = rcu_dereference(sdata->some_sta);
> +	if (some_sta) {
> +		if (WARN_ON(some_sta->sdata != sdata))
> +			rcu_assign_pointer(sdata->some_sta, NULL);
> +		else
> +			if (ether_addr_equal(some_sta->sta.addr, addr))
> +				return some_sta;

I worry a little bit about the overhead in the "always cache miss" case.
Is this really helpful for AP interfaces? Maybe it should be limited to
managed virtual interfaces.

>  
>  	sta = rcu_dereference_check(local->sta_hash[STA_HASH(addr)],
>  				    lockdep_is_held(&local->sta_mtx));
> @@ -263,10 +273,14 @@ struct sta_info *sta_info_get_by_idx(struct ieee80211_sub_if_data *sdata,
>   */
>  void sta_info_free(struct ieee80211_local *local, struct sta_info *sta)
>  {
> +	struct sta_info* some_sta;
>  	if (sta->rate_ctrl)

missing blank line

>  		rate_control_free_sta(sta);
>  
>  	sta_dbg(sta->sdata, "Destroyed STA %pM\n", sta->sta.addr);
> +	some_sta = rcu_dereference(sta->sdata->some_sta);
> +	if (some_sta == sta)
> +		rcu_assign_pointer(sta->sdata->some_sta, NULL);

This clearing is WAY too late. You can look up the station way after you
must be allowed to, this will invariably crash eventually. It absolutely
must be when the station is unhashed, not when it's freed.

> @@ -373,6 +387,8 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata,
>  	for (i = 0; i < NUM_RX_DATA_QUEUES; i++)
>  		sta->last_seq_ctrl[i] = cpu_to_le16(USHRT_MAX);
>  
> +	rcu_assign_pointer(sta->sdata->some_sta, sta);

This is too early, the station might not even be used, this must only be
done when the station is hashed.

johannes

--
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


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux