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

[Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>]

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.

It would be useful to add a Tested-on: tag if this was validated on
affected hardware, as suggested in the ath11k submission guidelines.

> 
> Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")

Since the full_mon side was added later by 7e2ea2e94704, it may be worth 
adding that as a second Fixes: tag for completeness.

> 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;
>   

The code change itself looks correct to me.

Reviewed-by: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>