Re: [PATCH 17/17] net/dpaa: improve dpaa errata A010022 handling

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 8/1/2024 11:53 AM, Hemant Agrawal wrote:
> From: Jun Yang <jun.yang@nxp.com>
> 
> This patch improves the errata handling for
> "RTE_LIBRTE_DPAA_ERRATA_LS1043_A010022"
> 
> Signed-off-by: Jun Yang <jun.yang@nxp.com>
> ---
>  drivers/net/dpaa/dpaa_rxtx.c | 40 ++++++++++++++++++++++++++++--------
>  1 file changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/dpaa/dpaa_rxtx.c b/drivers/net/dpaa/dpaa_rxtx.c
> index 84fd0c57a4..325785480a 100644
> --- a/drivers/net/dpaa/dpaa_rxtx.c
> +++ b/drivers/net/dpaa/dpaa_rxtx.c
> @@ -1264,6 +1264,35 @@ reallocate_mbuf(struct qman_fq *txq, struct rte_mbuf *mbuf)
>  	return new_mbufs[0];
>  }
>  
> +#ifdef RTE_LIBRTE_DPAA_ERRATA_LS1043_A010022
> +/* In case the data offset is not multiple of 16,
> + * FMAN can stall because of an errata. So reallocate
> + * the buffer in such case.
> + */
> +static inline int
> +dpaa_eth_ls1043a_mbuf_realloc(struct rte_mbuf *mbuf)
> +{
> +	uint64_t len, offset;
> +
> +	if (dpaa_svr_family != SVR_LS1043A_FAMILY)
> +		return 0;
> +
> +	while (mbuf) {
> +		len = mbuf->data_len;
> +		offset = mbuf->data_off;
> +		if ((mbuf->next &&
> +			!rte_is_aligned((void *)len, 16)) ||
> +			!rte_is_aligned((void *)offset, 16)) {
> +			DPAA_PMD_DEBUG("Errata condition hit");
> +
> +			return 1;
> +		}
> +		mbuf = mbuf->next;
> +	}
> +	return 0;
> +}
> +#endif
> +
>  uint16_t
>  dpaa_eth_queue_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
>  {
> @@ -1304,20 +1333,15 @@ dpaa_eth_queue_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
>  				DPAA_TX_BURST_SIZE : nb_bufs;
>  		for (loop = 0; loop < frames_to_send; loop++) {
>  			mbuf = *(bufs++);
> -			/* In case the data offset is not multiple of 16,
> -			 * FMAN can stall because of an errata. So reallocate
> -			 * the buffer in such case.
> -			 */
> -			if (dpaa_svr_family == SVR_LS1043A_FAMILY &&
> -					(mbuf->data_off & 0x7F) != 0x0)
> -				realloc_mbuf = 1;
> -
>

Previous approach seems better, detect when need to apply this errata,
and apply automatically.

Now there is a macro controlling it, how a user will know this errata is
required and how to enable it. And this prevent same binary distributed
multiple environments combination of errata required and not required.

Why not stick to the old method?

>  			fd_arr[loop].cmd = 0;
>  #if defined(RTE_LIBRTE_IEEE1588)
>  			fd_arr[loop].cmd |= DPAA_FD_CMD_FCO |
>  				qman_fq_fqid(fq_txconf);
>  			fd_arr[loop].cmd |= DPAA_FD_CMD_RPD |
>  				DPAA_FD_CMD_UPD;
> +#endif
> +#ifdef RTE_LIBRTE_DPAA_ERRATA_LS1043_A010022
> +			realloc_mbuf = dpaa_eth_ls1043a_mbuf_realloc(mbuf);
>  #endif
>  			seqn = *dpaa_seqn(mbuf);
>  			if (seqn != DPAA_INVALID_MBUF_SEQN) {