RE: [PATCH] rtw88: add debugfs to fix tx rate

[Tony Chuang <yhchuang@realtek.com>]

Ben Greear <greearb@candelatech.com> writes:

> On 3/17/20 8:40 AM, Kalle Valo wrote:
>> Tony Chuang <yhchuang@realtek.com> writes:
>>
>>> // Add Johannes for commenting on adding another nl80211 commands
>>>
>>> Kalle Valo <kvalo@codeaurora.org> writes:>
>>>
>>>> Tony Chuang <yhchuang@realtek.com> writes:
>>>>
>>>>> Kalle Valo <kvalo@codeaurora.org> writes:
>>>>>
>>>>>> <yhchuang@realtek.com> writes:
>>>>>>
>>>>>>> From: Yan-Hsuan Chuang <yhchuang@realtek.com>
>>>>>>>
>>>>>>> It is useful to fix the bit rate of TX packets. For example, if
>>>>>>> someone is measuring the TX power, or debugging with the issues
>>>>>>> of the TX throughput on the field.
>>>>>>>
>>>>>>> To set the value of fixed rate, one should input corresponding
>>>>>>> desc rate index (ex, 0x0b for DESC_RATE54M to fix at 54 Mbps).
>>>>>>> Set a value larger than DESC_RATE_MAX will disable fix rate, so
>>>>>>> the rate adaptive mechanism can resume to work.
>>>>>>>
>>>>>>> Example,
>>>>>>>    To fix rate at MCS 1:
>>>>>>>    echo 0x0d > /sys/kernel/debug/ieee80211/phy0/rtw88/fix_rate
>>>>>>>
>>>>>>>    To not to fix rate:
>>>>>>>    echo 0xff > /sys/kernel/debug/ieee80211/phy0/rtw88/fix_rate
>>>>>>>
>>>>>>>    To know which rate was fixed at:
>>>>>>>    cat /sys/kernel/debug/ieee80211/phy0/rtw88/fix_rate
>>>>>>>
>>>>>>> Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
>>>>>>
>>>>>> No way, debugfs is not a method for working around nl80211 and
>doing
>>>>>> whatever idea you come up with. The goal is that we have a generic
>>>>>> nl80211 command for all generic actions, like this one. And I think we
>>>>>> already have an nl80211 command for fixing the tx rate, right?
>>>>>>
>>>>>
>>>>> No, as I can see, there's no suitable nl80211 command that can achieve
>>>>> what I want. If you are saying about
>>>> NL80211_CMD_SET_TX_BITRATE_MASK,
>>>>> it's used to allow some rates. But actually the firmware has its own rate
>>>>> adaptive mechanism, so mask out the other rates does not mean the
> rate
>>>>> left will be chosen. Moreover, the hardware will choose a lower bit rate
>>>>> when retry, then the TX rate is not fixed at all. So the debugfs can
> disable
>>>>> the firmware's RA mechanism, also disable the TX rate fall back when
> retry.
>>>>> Both of them cannot be done by setting TX bitrate mask.


This is the reason the nl80211 command is not suitable for me.


>>>>
>>>> I'm confused, here you talk about firmware implementation etc but I'm
>>>> just talking about replacing the fix_rate debugfs file to an nl80211
>>>> command (for providing the fix_rate value). Can you clarify more why
> you
>>>> think nl80211 is not suitable?
>>>
>>> Oops, I thought that you wanted me to use the existing nl80211
>>> command.
>>
>> Either use an existing nl80211 command or add a new one if needed. For
>> me most important is that we don't add hacks to debugfs just for
>> avoiding using nl80211.
>>
>>> Now I know that you think we can add a new nl80211 command to help
>>> drivers to fix the TX bitrate if necessary. If adding another nl80211
>>> command for that is acceptable, I can work on this. But I need
>>> Johannes's comment if it's better to add a new nl80211 command or to
>>> expand the existing command (ex.
> NL80211_CMD_SET_TX_BITRATE_MASK).
>>
>> _Why_ is NL80211_CMD_SET_TX_BITRATE_MASK not suitable for you? You
> keep
>> saying that but I have still figured out why exactly you think so.
>> Please clarify this in detail.

I think I've talked about it in my previous mail, see above.

This command just mask out some of rates that are not allowed. But the
firmware has its own rate adaptive mechanism to choose the rates. So mask
out all of the other rate doesn't make sure the packets will be transmitted by
the only rate that was not masked. The hardware/firmware will try to choose
a better rate (ex. 1Mbps or 6Mbps) if they think it's necessary. Also the device
will fallback the rates to try to find a better rate to transfer data to the peer.

>>
>>> It looks like that adding a new nl80211 command will be better for me
>>> as expanding the existing one would have great impact on the already
>>> distributed drivers/user-tools.
>>
>> What kind of great impact are you talking about? Please be specific so
>> that we don't need to guess.

We probably have to modify the command parser, from user-space and the
nl80211 domain, because as far I don't see a good way to add fix rate
option on the NL80211_CMD_SET_TX_BITRATE_MASK without changing
the existing mechanism. If the mechanism is changed, then the "old" drivers
will fail to interpret the nl80211 attributes. So I think add a new one, which
can fix the TX rate, disable the rate adaptive, etc., will be better if necessary.

> 
> At least with ath10k, the issues I found were that nl80211 doesn't like it
> when you try to disable all legacy rates (and force frames out at 54Mbps
> encoding, for instance).
> 
> I'm not even sure upstream ath10k will even let you set a single rate
> using normal API now.  Have you tried it?
> 
> Another problem is that to keep a connection alive, you probably want mgt
> and null-func frames to go out normal and only have the firmware use a
> particular MCS
> for data frames.
> 
> Lots of reasons to want a low-level hack for this sort of thing.

Thank you for point them out. Control the TX rate is really important when
debugging with issues on the field, especially when the air is noisy and the
rate adaptive mechanism is not working well. Because usually the device
tries to fallback the rate for stability, when lower bitrate can make the peer
have a higher opportunity to successfully receive the packet. When the rate
fallbacks, the rate is not fixed at all. And when we want a rate that is "fixed",
we don't want another rate appear in the air.

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

Yen-Hsuan