Re: [PATCH 1/2] ath9k: use ieee80211_tx_status_noskb where possible

[Felix Fietkau <nbd@nbd.name>]

On 2016-09-08 13:41, Kalle Valo wrote:
> Zefir Kurtisi <zefir.kurtisi@neratec.com> writes:
> 
>> On 08/11/2016 09:27 PM, Felix Fietkau wrote:
>>> On 2016-08-11 18:05, Zefir Kurtisi wrote:
>>>> On 08/04/2016 11:49 PM, Felix Fietkau wrote:
>>>>> It removes the need for undoing the padding changes to skb->data and it
>>>>> improves performance by eliminating one tx status lookup per MPDU in the
>>>>> status path. It is also useful for preparing a follow-up fix to better
>>>>> handle powersave filtering.
>>>>>
>>>>
>>>> For me, this one introduces a regression to the statistics, e.g.
>>>> 'dot11TransmittedFragmentCount' is now accounted differently since it is not
>>>> updated from within ieee80211_tx_status_noskb().
>>> Is this important? I guess it would be possible to make this more
>>> accurate by extending the API, but I wonder if that's worth doing just
>>> for these debugfs counters.
>>> 
>> If you want to support the IEEE802dot11.MIB (dot11mac.dot11CountersTable), they
>> are important. Not sure though if it is used by others besides us.
>>
>> I think the real issue here is that those counters are regarded as internal debug
>> values (as the comments or e.g. commit c206ca6709 state) instead of general
>> purpose statistics that is exposed to SNMP. As such, they should be configurable
>> as a feature independent of debug settings in mac80211.
>>
>>
>> Aside from that consideration, this patch (with the follow up ones) increased peak
>> performance noticeably (we measure some 5%+ higher peak throughput), which for
>> sure is worth dropping the counters for most.
>>
>> I am fine handling this internally. A note in the commit message would be helpful
>> that states that counters dot11TransmittedFragmentCount, dot11FrameDuplicateCount,
>> dot11ReceivedFragmentCount, and dot11MulticastReceivedFrameCount become invalid.
> 
> A good idea, I updated the commit log to mention that. Does that look
> ok?
> 
> Author: Felix Fietkau <nbd@nbd.name>
> Date:   Fri Sep 2 19:46:12 2016 +0300
> 
>     ath9k: use ieee80211_tx_status_noskb where possible
>     
>     It removes the need for undoing the padding changes to skb->data and it
>     improves performance by eliminating one tx status lookup per MPDU in the
>     status path. It is also useful for preparing a follow-up fix to better
>     handle powersave filtering.
>     
>     A side effect is that these counters, available via debugfs, become now invalid:
>     
>     * dot11TransmittedFragmentCount
>     * dot11FrameDuplicateCount,
>     * dot11ReceivedFragmentCount
>     * dot11MulticastReceivedFrameCount
>     
>     Signed-off-by: Felix Fietkau <nbd@nbd.name>
Looks good to me.

- Felix