RE: [PATCH rtw-next v5 02/10] wifi: rtw89: handle negative rate index for injected frames

[Ping-Ke Shih]

Enver Ikromov <owner@xxxxxxxxxxxxxx> wrote:
> When the radiotap header of an injected frame carries no rate,
> info->control.rates[0].idx is -1.

> rtw89_core_tx_update_injection()
> stores the index in a u8, so the value becomes 0xff and is added to
> the CCK1/OFDM6 base, programming an invalid hardware rate into the TX
> descriptor.

I don't prefer to describe function name in commit message. Just
describe the problem you encountered and how to fix it.

For this patch, you want to handle the rate idx < 0 case, and solution
is set rate to 1M and 6M for 2GHz band and others respectively, right?

I guess LLM help to generate this. Code flow is not important to
commit message. The patch should explain the flow itself.

> 
> Use a signed index. If it is negative, keep the rate already selected
> by rtw89_core_tx_update_mgmt_info() for management frames, and fall
> back to data_retry_lowest_rate (or OFDM6 when
> IEEE80211_TX_CTL_NO_CCK_RATE is set) for data frames.
> 
> Fixes: e96abe4fd7cd ("wifi: rtw89: consider data rate/bandwidth/GI for injected packets")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Enver Ikromov <owner@xxxxxxxxxxxxxx>
> ---
>  drivers/net/wireless/realtek/rtw89/core.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c
> index b7df97e..527593e 100644
> --- a/drivers/net/wireless/realtek/rtw89/core.c
> +++ b/drivers/net/wireless/realtek/rtw89/core.c
> @@ -1239,12 +1239,25 @@ static void rtw89_core_tx_update_injection(struct rtw89_dev *rtwdev,
>         enum mac80211_rate_control_flags flags = info->control.rates[0].flags;
>         struct rtw89_tx_desc_info *desc_info = &tx_req->desc_info;
>         const struct rtw89_chan *chan;
> -       u8 idx = info->control.rates[0].idx;
> +       s8 idx = info->control.rates[0].idx;
>         u8 nss, mcs;
> 
>         desc_info->use_rate = true;
>         desc_info->dis_data_fb = true;
> 
> +       /* No rate was given in the radiotap header. Management frames
> +        * already carry the rate from rtw89_core_tx_update_mgmt_info(), so
> +        * only data frames need a fallback.
> +        */
> +       if (idx < 0) {
> +               if (tx_req->tx_type == RTW89_CORE_TX_TYPE_DATA)
> +                       desc_info->data_rate =
> +                               info->flags & IEEE80211_TX_CTL_NO_CCK_RATE ?
> +                               RTW89_HW_RATE_OFDM6 :
> +                               desc_info->data_retry_lowest_rate;
> +               return;
> +       }
> +

How about just set idx = 0 if idx < 0? The existing code can select valid rate,
since any rate is well for the case idx < 0.

>         if (flags & IEEE80211_TX_RC_160_MHZ_WIDTH)
>                 desc_info->data_bw = 3;
>         else if (flags & IEEE80211_TX_RC_80_MHZ_WIDTH)
> --
> 2.56.0.windows.4
>