Re: [PATCH wireless] wifi: ath11k: fix monitor mode frame length by using correct descriptor size

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

On 4/29/2026 2:52 AM, Praneesh P wrote:
> On 4/7/2026 8:18 AM, Joshua Klinesmith wrote:
>> The monitor mode RX path in ath11k_dp_rx_mon_mpdu_pop() and
>> ath11k_dp_rx_full_mon_mpdu_pop() uses sizeof(struct hal_rx_desc) to
>> compute the packet buffer offset. This is the size of the union of all
>> chip-specific descriptors (the maximum), not the actual descriptor size
>> for the running chip. The later ath11k_dp_rx_msdus_set_payload() then
>> strips only hw_params.hal_desc_sz bytes (the chip-specific size) from
>> the front of the skb.
>>
>> On IPQ8074 and QCN9074, sizeof(struct hal_rx_desc) is 392 but
>> hal_desc_sz is 384, leaving 8 extra bytes of descriptor data at the
>> end of every monitor mode frame delivered to userspace. On WCN6855 the
>> sizes happen to match so the bug is not visible.
>>
>> The same mismatch in ath11k_dp_mon_set_frag_len() causes incorrect
>> fragment length calculation for multi-buffer MSDUs, under-counting
>> intermediate fragments by 8 bytes and over-counting the last fragment.
>>
>> Fix by using ar->ab->hw_params.hal_desc_sz consistently in both
>> monitor mpdu_pop functions and passing it through to set_frag_len.
>>
>> Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")
>> Link: https://github.com/openwrt/openwrt/issues/16183
>> Signed-off-by: Joshua Klinesmith <joshuaklinesmith@gmail.com>
>> ---
>>   drivers/net/wireless/ath/ath11k/dp_rx.c | 27 ++++++++++++++-----------
>>   1 file changed, 15 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
>> index 85defe11750d..c86ffc203f15 100644
>> --- a/drivers/net/wireless/ath/ath11k/dp_rx.c
>> +++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
>> @@ -4511,10 +4511,11 @@ int ath11k_dp_rx_pdev_alloc(struct ath11k_base *ab, int mac_id)
>>   	return 0;
>>   }
>>   
>> -static void ath11k_dp_mon_set_frag_len(u32 *total_len, u32 *frag_len)
>> +static void ath11k_dp_mon_set_frag_len(u32 *total_len, u32 *frag_len,
>> +				       u32 hal_desc_sz)
>>   {
>> -	if (*total_len >= (DP_RX_BUFFER_SIZE - sizeof(struct hal_rx_desc))) {
>> -		*frag_len = DP_RX_BUFFER_SIZE - sizeof(struct hal_rx_desc);
>> +	if (*total_len >= (DP_RX_BUFFER_SIZE - hal_desc_sz)) {
>> +		*frag_len = DP_RX_BUFFER_SIZE - hal_desc_sz;
>>   		*total_len -= *frag_len;
>>   	} else {
>>   		*frag_len = *total_len;
>> @@ -4658,19 +4659,19 @@ static u32 ath11k_dp_rx_mon_comp_ppduid(u32 msdu_ppdu_id, u32 *ppdu_id,
>>   
>>   static void ath11k_dp_mon_get_buf_len(struct hal_rx_msdu_desc_info *info,
>>   				      bool *is_frag, u32 *total_len,
>> -				      u32 *frag_len, u32 *msdu_cnt)
>> +				      u32 *frag_len, u32 *msdu_cnt,
>> +				      u32 hal_desc_sz)
>>   {
>>   	if (info->msdu_flags & RX_MSDU_DESC_INFO0_MSDU_CONTINUATION) {
>>   		if (!*is_frag) {
>>   			*total_len = info->msdu_len;
>>   			*is_frag = true;
>>   		}
>> -		ath11k_dp_mon_set_frag_len(total_len,
>> -					   frag_len);
>> +		ath11k_dp_mon_set_frag_len(total_len, frag_len, hal_desc_sz);
>>   	} else {
>>   		if (*is_frag) {
>> -			ath11k_dp_mon_set_frag_len(total_len,
>> -						   frag_len);
>> +			ath11k_dp_mon_set_frag_len(total_len, frag_len,
>> +						   hal_desc_sz);
>>   		} else {
>>   			*frag_len = info->msdu_len;
>>   		}
>> @@ -4792,7 +4793,7 @@ u32 ath11k_dp_rx_mon_mpdu_pop(struct ath11k *ar, int mac_id,
>>   
>>   			rx_desc = (struct hal_rx_desc *)msdu->data;
>>   
>> -			rx_pkt_offset = sizeof(struct hal_rx_desc);
>> +			rx_pkt_offset = ar->ab->hw_params.hal_desc_sz;
>>   			l2_hdr_offset = ath11k_dp_rx_h_msdu_end_l3pad(ar->ab, rx_desc);
>>   
>>   			if (is_first_msdu) {
>> @@ -4823,7 +4824,8 @@ u32 ath11k_dp_rx_mon_mpdu_pop(struct ath11k *ar, int mac_id,
>>   			}
>>   			ath11k_dp_mon_get_buf_len(&msdu_list.msdu_info[i],
>>   						  &is_frag, &total_len,
>> -						  &frag_len, &msdu_cnt);
>> +						  &frag_len, &msdu_cnt,
>> +						  rx_pkt_offset);
>>   			rx_buf_size = rx_pkt_offset + l2_hdr_offset + frag_len;
>>   
>>   			ath11k_dp_pkt_set_pktlen(msdu, rx_buf_size);
>> @@ -5424,7 +5426,7 @@ ath11k_dp_rx_full_mon_mpdu_pop(struct ath11k *ar,
>>   
>>   			rx_desc = (struct hal_rx_desc *)msdu->data;
>>   
>> -			rx_pkt_offset = sizeof(struct hal_rx_desc);
>> +			rx_pkt_offset = ar->ab->hw_params.hal_desc_sz;
>>   			l2_hdr_offset = ath11k_dp_rx_h_msdu_end_l3pad(ar->ab, rx_desc);
>>   
>>   			if (is_first_msdu) {
>> @@ -5439,7 +5441,8 @@ ath11k_dp_rx_full_mon_mpdu_pop(struct ath11k *ar,
>>   
>>   			ath11k_dp_mon_get_buf_len(&msdu_list.msdu_info[i],
>>   						  &is_frag, &total_len,
>> -						  &frag_len, &msdu_cnt);
>> +						  &frag_len, &msdu_cnt,
>> +						  rx_pkt_offset);
>>   
>>   			rx_buf_size = rx_pkt_offset + l2_hdr_offset + frag_len;
> 
> Thanks for fixing the offset handling in the monitor Rx path, but still 
> there is another instance that still relies on sizeof(struct hal_rx_desc).
> 
> ath11k_dp_rx_h_ppdu(), which is also invoked from the monitor path, uses:
> 
> ath11k_dbg_dump(ab, ATH11K_DBG_DATA, NULL, "", rx_desc, sizeof(struct 
> hal_rx_desc));
> 
> This should likewise be converted to use hw_params.hal_desc_sz to avoid 
> dumping beyond the chip-specific descriptor size on platforms where they 
> differ.

Joshua, can you submit a v2 that addresses this observation?

/jeff