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

[Hemant Agrawal <hemant.agrawal@oss.nxp.com>]

On 07-08-2024 21:11, Ferruh Yigit wrote:
> 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?

Currently only the LS1043 platform is effected by this errata. Other 
DPAA paltforms are not impacted.

So, only LS1043 customers, if they are using impacted scenario need to 
enable it. Other DPAA SoC customers will not be impacted

by these additional checks in datapath.


>
>>   			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) {