Received-SPF: pass (google.com: domain of linux-wireless+bounces-6154-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: Kalle Valo <kvalo@kernel.org>
To: Nithyanantham P <quic_nithp@quicinc.com>
Cc: <ath12k@lists.infradead.org>,  <linux-wireless@vger.kernel.org>,
  Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
Subject: Re: [PATCH v2] wifi: ath12k: fix mac id extraction when MSDU
 spillover in rx error path
References: <20240329092432.873710-1-quic_nithp@quicinc.com>
Date: Thu, 11 Apr 2024 12:33:43 +0300
In-Reply-To: <20240329092432.873710-1-quic_nithp@quicinc.com> (Nithyanantham
	P.'s message of "Fri, 29 Mar 2024 14:54:32 +0530")
Message-ID: <87bk6gz1jc.fsf@kernel.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

Nithyanantham P <quic_nithp@quicinc.com> writes:

> From: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
>
> Currently, in the rx error data path, mac id is extracted from the
> last 64bits of MSDU end description tag for each entry received in
> the WBM error ring. Then, each entry will be updated into the MSDU
> list for further processing. The extracted mac id is valid when a
> single MSDU is not fragmented and received in the WBM error ring.
>
> In scenarios where the size of a single MSDU received exceeds the
> descriptor buffer size, resulting in fragmented or spillover MSDU
> entries into the WBM error ring. In this case, the extracted mac id
> from each spillover entry is invalid except the last spillover entry
> of the MSDU. This invalid mac id leads to packet rejection.
>
> To address this issue, check if the MSDU continuation flag is set,
> then extract the valid mac id from the last spillover entry.
> Propagate the valid mac id to all the spillover entries of the single
> MSDU in the temporary MSDU list(scatter_msdu_list). Then, update this
> into the MSDU list (msdu_list) for further processing.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
> Signed-off-by: Nithyanantham P <quic_nithp@quicinc.com>

[...]

> @@ -3807,16 +3816,50 @@ int ath12k_dp_rx_process_wbm_err(struct ath12k_base *ab,
>  			continue;
>  		}
>  
> +		msdu_data = (struct hal_rx_desc *)msdu->data;
>  		rxcb->err_rel_src = err_info.err_rel_src;
>  		rxcb->err_code = err_info.err_code;
> -		rxcb->rx_desc = (struct hal_rx_desc *)msdu->data;
> -
> -		__skb_queue_tail(&msdu_list, msdu);
> -
>  		rxcb->is_first_msdu = err_info.first_msdu;
>  		rxcb->is_last_msdu = err_info.last_msdu;
>  		rxcb->is_continuation = err_info.continuation;
> +		rxcb->rx_desc = msdu_data;
> +
> +		if (err_info.continuation) {
> +			__skb_queue_tail(&scatter_msdu_list, msdu);
> +		} else {
> +			mac_id = ath12k_dp_rx_get_msdu_src_link(ab,
> +								msdu_data);
> +			if (mac_id >= MAX_RADIOS) {
> +				dev_kfree_skb_any(msdu);
> +
> +				/* In any case continuation bit is set
> +				 * in the previous record, cleanup scatter_msdu_list
> +				 */
> +				ath12k_dp_clean_up_skb_list(&scatter_msdu_list);
> +				continue;
> +			}
> +
> +			if (!skb_queue_empty(&scatter_msdu_list)) {
> +				struct sk_buff *msdu;
> +
> +				skb_queue_walk(&scatter_msdu_list, msdu) {
> +					rxcb = ATH12K_SKB_RXCB(msdu);
> +					rxcb->mac_id = mac_id;
> +				}
> +
> +				skb_queue_splice_tail_init(&scatter_msdu_list,
> +							   &msdu_list);
> +			}
> +
> +			rxcb = ATH12K_SKB_RXCB(msdu);
> +			rxcb->mac_id = mac_id;
> +			__skb_queue_tail(&msdu_list, msdu);
> +		}
>  	}

The else branch can be avoided with continue. I did that in the pending
branch, please check my changes:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=6c677d91adad4817e7f6ef65a85331f52f0237ee

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches