Re: [RFC 1/2] mac80211: Add vif hash for multi-station RX performance.

[Ben Greear <greearb@candelatech.com>]

On 04/11/2013 02:19 AM, Johannes Berg wrote:
> On Tue, 2013-04-09 at 10:54 -0700, Ben Greear wrote:
>
>>>> +struct sta_info *sta_info_get_by_vif(struct ieee80211_local *local,
>>>> +				     const u8 *vif_addr, const u8 * sta_addr) {
>>>> +	struct sta_info *sta;
>>>> +
>>>> +	sta = rcu_dereference_check(local->sta_vhash[STA_HASH(vif_addr)],
>>>> +				    lockdep_is_held(&local->sta_mtx));
>>>> +	while (sta) {
>>>> +		if (ether_addr_equal(sta->sdata->vif.addr, vif_addr) &&
>>>> +		    ether_addr_equal(sta->sta.addr, sta_addr))
>>>> +			break;
>>>> +		sta = rcu_dereference_check(sta->vnext,
>>>> +					    lockdep_is_held(&local->sta_mtx));
>>>
>>> Almost all of your rcu_dereference_check() invocations should be
>>> rcu_dereference_protected(). See include/linux/rcupdate.h :)
>>
>> Now this, I'm not so sure of.  That rcu_dereference_protected seems to
>> be only used for the 'update-side' use.  I was under the impression
>> that when the mac80211 rx logic is called we are only protected by rcu,
>> not the update mutex.
>
> Ah, yes, I was reading this the wrong way, sorry. Here the _check() is
> correct of course -- you want to be either under RCU protection or hold
> the sta_mtx.
>
>> I also struggle to understand RCU properly...so maybe I'm just
>> wrong about all that...
>>
>> The other methods to get sta_info around that code use the _check() variant,
>> by the way...
>
> Yeah ... :)
>
> Another question: Have you thought about hashing the virtual interfaces
> instead of the stations, and then hashing the stations inside each
> virtual interface? That would make it a bit of a two-level thing:
>
> A1 (in the frame) -> virtual interface
>      A2 (frame) -> station
>
> But it would address the TX side efficiently without "some_sta" since
> you know the virtual interface there already, and could potentially have
> less impact on the code? On TX it'd actually even be more efficient if
> you have more than 1 station per interface (right now you don't though)

This idea suddenly looks a lot more interesting.  The ieee80211_tx_status method needs
to find the remote station & sdata, but in the AP case, the station hash works best,
and in my many-sta-vif case, the VIF hash works best.  I don't see any way to guess
which hash to use in this case.

But, if we first hashed to find sdata, and then had a vif hash in the sdata
object, the lookup should be fast for cases where the hash function works
well.

I'll give this a try...

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com