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

Message ID 20240801105313.630280-18-hemant.agrawal@nxp.com (mailing list archive)
State Changes Requested
Delegated to: Ferruh Yigit
Headers
Series NXP DPAA ETH driver enhancement and fixes |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-marvell-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-sample-apps-testing success Testing PASS

Commit Message

Hemant Agrawal Aug. 1, 2024, 10:53 a.m. UTC
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(-)
  

Comments

Ferruh Yigit Aug. 7, 2024, 3:41 p.m. UTC | #1
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) {
  
Hemant Agrawal Aug. 23, 2024, 7:35 a.m. UTC | #2
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) {
  

Patch

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