Re: [ath9k-devel] ath9k, multiple stations, and AMPDUs

[Felix Fietkau <nbd@openwrt.org>]

On 2010-09-22 6:33 AM, Ben Greear wrote:
> On 09/21/2010 03:41 PM, Felix Fietkau wrote:
>> On 2010-09-21 10:19 PM, Ben Greear wrote:
>>> On 09/21/2010 12:32 PM, Felix Fietkau wrote:
>>>> On 2010-09-21 9:28 PM, Johannes Berg wrote:
>>>>> On Tue, 2010-09-21 at 20:00 +0200, Felix Fietkau wrote:
>>>>>
>>>>>>> Could we just poke a pointer to the STA into the ath_buf structure?
>>>>>
>>>>>> No, that doesn't work because of RCU.
>>>>>
>>>>> Well, it could work, if you walk all the structures upon sta_notify and
>>>>> remove now stale pointers (or just drop the frames or something).
>>>> I think it would be much better to just add the helper function that
>>>> checks the RA on STA lookup. Keeps things simple, especially since
>>>> nothing else in the tx path needs the vif.
>>>
>>> How about this.  Seems to do the trick on my system:
>>>
>>>
>>> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
>>> index 85a7323..09815a1 100644
>>> --- a/drivers/net/wireless/ath/ath9k/xmit.c
>>> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
>>> @@ -328,8 +328,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
>>>
>>>           rcu_read_lock();
>>>
>>> -       /* XXX: use ieee80211_find_sta! */
>>> -       sta = ieee80211_find_sta_by_hw(hw, hdr->addr1);
>>> +       sta = tx_info->control.sta;
>> As I mentioned in another email: at the time we get the tx status
>> report, we have to consider the sta pointer stale. It may or may not
>> still be valid.
> 
> How about this one.  I think it ensures that the sta will never be stale,
No, it doesn't. At least not in AP mode.

> since it flushes the tx queue on vif removal.  Minimal testing shows it
> working, but of course I might be missing something.
In AP mode, a vif has multiple sta. And draining the queue when a sta
gets removed is not a good idea.

- Felix