Re: [PATCH] ath9k: fix per-packet TX-power cap for TPC

["Toke Høiland-Jørgensen" <toke@kernel.org>]

Jonas Jelonek <jelonek.jonas@gmail.com> writes:

>> On 30. Mar 2023, at 11:31, Toke Høiland-Jørgensen <toke@kernel.org> wrote:
>> 
>> Jonas Jelonek <jelonek.jonas@gmail.com> writes:
>> 
>>> Fix incorrect usage of plain rate_idx as index into the max (power) per
>>> rate lookup table.
>>> 
>>> For transmit power control (TPC), the ath9k driver maintains internal
>>> tables (in struct ath_hw) to store the max allowed power level per rate.
>>> They are used to limit a given TX-power according to regulatory and user
>>> limits in the TX-path per packet. The tables are filled in a predefined
>>> order, starting with values for CCK + OFDM rates and followed by the
>>> values for MCS rates. Thus, the maximum power levels for MCS do not
>>> start at index 0 in the table but are shifted by a fixed value.
>>> 
>>> The TX-power limiting in ath_get_rate_txpower did not apply this shift,
>>> thus retrieved the incorrect maximum power level. For example, the
>>> maximum power for OFDM rate 0 was used for MCS rate 0. If STBC was used,
>>> the power was mostly limited to 0 because the STBC table is zeroed for
>>> legacy CCK/OFDM rates. This patch fixes this table lookup.
>>> 
>>> Signed-off-by: Jonas Jelonek <jelonek.jonas@gmail.com>
>> 
>> So what effect does this bug have in practice? Also, how did you test
>> the patch? :)
>> 
>
> It actually may not have an effect in most of current practical applications. The
> member ‘tpc_enabled' in struct ath_hw is by default set to false and thus, TPC
> in ath9k is usually disabled. The code path will be skipped in that case. 
> But it has an effect as I am still working on TPC per packet in ath9k as part of
> research. Looking at my traces I saw that the TX-power is capped at 0 for
> MCS rates 0-7 in case the STBC flag is set in struct ieee80211_tx_info. Thus,
> transmission was significantly worse as usual, and I also was not able to 
> manually set a proper TX-power via my testing API. With other rates it is working
> fine. I also double-checked that it isn’t a problem of how TX-power is set and
> reported with our upcoming API.
>
> I tested this with my OpenWrt-based AP-STA desk setup using two different
> ath9k wifi chips (AR9280, AR9580), Kernel 5.15.98. I guess that should be
> sufficient as ath9k hasn’t changed that significantly since that and several
> changes are already backported. I compile-tested it with latest 6.3 kernel
> (+allyesconfig), the patch applied flawlessly and encountered no problems. 
> After applying my patch, I could see that everything was working as expected.
> Transmission performs equally as if TPC was disabled, and setting + reporting
> TX-power was working again. Also verified this with some verbose debugging
> in ath_get_rate_txpower to see which index + max power is used.

Alright, cool! Could you please add all this to the commit message and
resubmit?

-Toke