Re: [PATCH v2] wifi: ath12k: fix incorrect HT/VHT/HE/EHT MCS reporting in monitor mode

[Jeff Johnson <jeff.johnson@oss.qualcomm.com>]

On 5/5/2026 9:43 AM, Tamizh Raja wrote:
> On Tue, May 5, 2026 at 9:40 AM kwan1996 <laicheehou9@gmail.com> wrote:
>>
>> In monitor mode, the driver incorrectly assigns the legacy rate
>> to the rate_idx field of the radiotap header for HT/VHT/HE/EHT
>> frames, ignoring the actual MCS value parsed from the hardware.
>>
>> This causes packet analyzers (like Wireshark) to display incorrect
>> MCS values (e.g., legacy base rates instead of the true MCS).
>>
>> Fix this by assigning ppdu_info->mcs instead of ppdu_info->rate
>> for HT/VHT/HE/EHT frame types in ath12k_dp_mon_fill_rx_rate()
>> and ath12k_dp_mon_update_radiotap().
>>
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=220864
>>
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.4.1-00199-QCAHKSWPL_SILICONZ
>>
>> Signed-off-by: kwan1996 <laicheehou9@gmail.com>
>>
>> ---
>>
>> v2: Fix indentation and formatting issues in v1.
>>
>> ---
>>  drivers/net/wireless/ath/ath12k/dp_mon.c | 10 +++++++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/dp_mon.c b/drivers/net/wireless/ath/ath12k/dp_mon.c
>> index 39d1967..4119bb8 100644
>> --- a/drivers/net/wireless/ath/ath12k/dp_mon.c
>> +++ b/drivers/net/wireless/ath/ath12k/dp_mon.c
>> @@ -1925,6 +1925,7 @@ ath12k_dp_mon_fill_rx_rate(struct ath12k *ar,
>>                 }
>>                 break;
>>         case RX_MSDU_START_PKT_TYPE_11N:
>> +               rate_mcs = ppdu_info->mcs;
> 
> Can we assign this rate_mcs before the switch case? Since in all cases
> we are assigning unmodified ppdu_info->mcs.
>>                 rx_status->encoding = RX_ENC_HT;
>>                 if (rate_mcs > ATH12K_HT_MCS_MAX) {
>>                         ath12k_warn(ar->ab,
>> @@ -1937,6 +1938,7 @@ ath12k_dp_mon_fill_rx_rate(struct ath12k *ar,
>>                         rx_status->enc_flags |= RX_ENC_FLAG_SHORT_GI;
>>                 break;
>>         case RX_MSDU_START_PKT_TYPE_11AC:
>> +               rate_mcs = ppdu_info->mcs;
>>                 rx_status->encoding = RX_ENC_VHT;
>>                 rx_status->rate_idx = rate_mcs;
>>                 if (rate_mcs > ATH12K_VHT_MCS_MAX) {
>> @@ -1949,6 +1951,7 @@ ath12k_dp_mon_fill_rx_rate(struct ath12k *ar,
>>                         rx_status->enc_flags |= RX_ENC_FLAG_SHORT_GI;
>>                 break;
>>         case RX_MSDU_START_PKT_TYPE_11AX:
>> +               rate_mcs = ppdu_info->mcs;
>>                 rx_status->rate_idx = rate_mcs;
>>                 if (rate_mcs > ATH12K_HE_MCS_MAX) {
>>                         ath12k_warn(ar->ab,
>> @@ -1960,6 +1963,7 @@ ath12k_dp_mon_fill_rx_rate(struct ath12k *ar,
>>                 rx_status->he_gi = ath12k_he_gi_to_nl80211_he_gi(sgi);
>>                 break;
>>         case RX_MSDU_START_PKT_TYPE_11BE:
>> +               rate_mcs = ppdu_info->mcs;
>>                 rx_status->rate_idx = rate_mcs;
>>                 if (rate_mcs > ATH12K_EHT_MCS_MAX) {
>>                         ath12k_warn(ar->ab,
>> @@ -2259,13 +2263,13 @@ static void ath12k_dp_mon_update_radiotap(struct ath12k *ar,
>>                 rxs->encoding = RX_ENC_HE;
>>                 ptr = skb_push(mon_skb, sizeof(struct ieee80211_radiotap_he));
>>                 ath12k_dp_mon_rx_update_radiotap_he(ppduinfo, ptr);
>> -               rxs->rate_idx = ppduinfo->rate;
>> +               rxs->rate_idx = ppduinfo->mcs;
>>         } else if (ppduinfo->vht_flags) {
>>                 rxs->encoding = RX_ENC_VHT;
>> -               rxs->rate_idx = ppduinfo->rate;
>> +               rxs->rate_idx = ppduinfo->mcs;
>>         } else if (ppduinfo->ht_flags) {
>>                 rxs->encoding = RX_ENC_HT;
>> -               rxs->rate_idx = ppduinfo->rate;
>> +               rxs->rate_idx = ppduinfo->mcs;
> 
> rate_idx should be assigned with ppdu_info->rate only not mcs.

why is that? documentation says:
 * @rate_idx: index of data rate into band's supported rates or MCS index if
 *	HT or VHT is used (%RX_FLAG_HT/%RX_FLAG_VHT)

ppduinfo contains separate rate and mcs so doesn't one or the other need to be
copied to rxs->rate_idx based upon the current PHY configuration?

btw looks like the struct ieee80211_rx_status documentation needs to be
updated for HE & EHT (and UHR?)