[dpdk-dev,PATCHv6,22/33] net/dpaa2: add support for l3 and l4 checksum offload

Message ID 1485172803-17288-23-git-send-email-hemant.agrawal@nxp.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel compilation success Compilation OK

Commit Message

Hemant Agrawal Jan. 23, 2017, 11:59 a.m. UTC
  Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 doc/guides/nics/features/dpaa2.ini      |  2 +
 drivers/bus/fslmc/portal/dpaa2_hw_pvt.h |  6 +++
 drivers/net/dpaa2/Makefile              |  2 +-
 drivers/net/dpaa2/dpaa2_ethdev.c        | 72 +++++++++++++++++++++++++++++++--
 4 files changed, 77 insertions(+), 5 deletions(-)
  

Comments

Ferruh Yigit Jan. 23, 2017, 5:35 p.m. UTC | #1
On 1/23/2017 11:59 AM, Hemant Agrawal wrote:
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
<...>
> --- a/drivers/net/dpaa2/Makefile
> +++ b/drivers/net/dpaa2/Makefile
> @@ -66,6 +66,6 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_eal lib/librte_ether
>  DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_mempool lib/librte_mbuf
>  DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_common_dpaa2_qbman
>  DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_bus_fslmc
> -DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_pmd_dpaa2_pool
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_pool_dpaa2

I understand these are added since DEPDIRS converted to LDLIBS, but this
does not looks right, since these variables mainly dependency solving
and the value provided is not correct for DEPDIRS.

Did you tried adding as "LDLIBS += xx", not tested, but may work.

>  
>  include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
> index 6de571a..2298246 100644
> --- a/drivers/net/dpaa2/dpaa2_ethdev.c
> +++ b/drivers/net/dpaa2/dpaa2_ethdev.c
> @@ -68,7 +68,17 @@
>  	dev_info->min_rx_bufsize = DPAA2_MIN_RX_BUF_SIZE;
>  	dev_info->max_rx_queues = (uint16_t)priv->nb_rx_queues;
>  	dev_info->max_tx_queues = (uint16_t)priv->nb_tx_queues;
> -
> +	dev_info->rx_offload_capa =
> +		DEV_RX_OFFLOAD_IPV4_CKSUM |
> +		DEV_RX_OFFLOAD_UDP_CKSUM |
> +		DEV_RX_OFFLOAD_TCP_CKSUM |
> +		DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM;

Is there capabilities always enabled for all devices this driver
support? Or should these be read from some device registers?

> +	dev_info->tx_offload_capa =
> +		DEV_TX_OFFLOAD_IPV4_CKSUM |
> +		DEV_TX_OFFLOAD_UDP_CKSUM |
> +		DEV_TX_OFFLOAD_TCP_CKSUM |
> +		DEV_TX_OFFLOAD_SCTP_CKSUM |
> +		DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM;
>  	dev_info->speed_capa = ETH_LINK_SPEED_1G |
>  			ETH_LINK_SPEED_2_5G |
>  			ETH_LINK_SPEED_10G;
<...>
  
Hemant Agrawal Jan. 24, 2017, 10:45 a.m. UTC | #2
On 1/23/2017 11:05 PM, Ferruh Yigit wrote:
> On 1/23/2017 11:59 AM, Hemant Agrawal wrote:
>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>> ---
> <...>
>> --- a/drivers/net/dpaa2/Makefile
>> +++ b/drivers/net/dpaa2/Makefile
>> @@ -66,6 +66,6 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_eal lib/librte_ether
>>  DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_mempool lib/librte_mbuf
>>  DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_common_dpaa2_qbman
>>  DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_bus_fslmc
>> -DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_pmd_dpaa2_pool
>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_pool_dpaa2
>
> I understand these are added since DEPDIRS converted to LDLIBS, but this
> does not looks right, since these variables mainly dependency solving
> and the value provided is not correct for DEPDIRS.
>
> Did you tried adding as "LDLIBS += xx", not tested, but may work.
>

Our intention was to create dependency to help in parallel build in case 
of shared library.  "LDLIBS += xx" also work, but we are still not able 
to create inter driver directory dependency for parallel build.


We may need your help in understanding the change required in depdir 
script to support directories within "driver".

>>
>>  include $(RTE_SDK)/mk/rte.lib.mk
>> diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
>> index 6de571a..2298246 100644
>> --- a/drivers/net/dpaa2/dpaa2_ethdev.c
>> +++ b/drivers/net/dpaa2/dpaa2_ethdev.c
>> @@ -68,7 +68,17 @@
>>  	dev_info->min_rx_bufsize = DPAA2_MIN_RX_BUF_SIZE;
>>  	dev_info->max_rx_queues = (uint16_t)priv->nb_rx_queues;
>>  	dev_info->max_tx_queues = (uint16_t)priv->nb_tx_queues;
>> -
>> +	dev_info->rx_offload_capa =
>> +		DEV_RX_OFFLOAD_IPV4_CKSUM |
>> +		DEV_RX_OFFLOAD_UDP_CKSUM |
>> +		DEV_RX_OFFLOAD_TCP_CKSUM |
>> +		DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM;
>
> Is there capabilities always enabled for all devices this driver
> support? Or should these be read from some device registers?
>
Capabilities are always enabled for all devices (MC DPNI object in this 
case)

>> +	dev_info->tx_offload_capa =
>> +		DEV_TX_OFFLOAD_IPV4_CKSUM |
>> +		DEV_TX_OFFLOAD_UDP_CKSUM |
>> +		DEV_TX_OFFLOAD_TCP_CKSUM |
>> +		DEV_TX_OFFLOAD_SCTP_CKSUM |
>> +		DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM;
>>  	dev_info->speed_capa = ETH_LINK_SPEED_1G |
>>  			ETH_LINK_SPEED_2_5G |
>>  			ETH_LINK_SPEED_10G;
> <...>
>
  
Ferruh Yigit Jan. 24, 2017, 10:51 a.m. UTC | #3
On 1/24/2017 10:45 AM, Hemant Agrawal wrote:
> On 1/23/2017 11:05 PM, Ferruh Yigit wrote:
>> On 1/23/2017 11:59 AM, Hemant Agrawal wrote:
>>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>>> ---
>> <...>
>>> --- a/drivers/net/dpaa2/Makefile
>>> +++ b/drivers/net/dpaa2/Makefile
>>> @@ -66,6 +66,6 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_eal lib/librte_ether
>>>  DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_mempool lib/librte_mbuf
>>>  DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_common_dpaa2_qbman
>>>  DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_bus_fslmc
>>> -DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_pmd_dpaa2_pool
>>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_pool_dpaa2
>>
>> I understand these are added since DEPDIRS converted to LDLIBS, but this
>> does not looks right, since these variables mainly dependency solving
>> and the value provided is not correct for DEPDIRS.
>>
>> Did you tried adding as "LDLIBS += xx", not tested, but may work.
>>
> 
> Our intention was to create dependency to help in parallel build in case 
> of shared library.  "LDLIBS += xx" also work, but we are still not able 
> to create inter driver directory dependency for parallel build.
> 
> 
> We may need your help in understanding the change required in depdir 
> script to support directories within "driver".
> 
>>>
>>>  include $(RTE_SDK)/mk/rte.lib.mk
>>> diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
>>> index 6de571a..2298246 100644
>>> --- a/drivers/net/dpaa2/dpaa2_ethdev.c
>>> +++ b/drivers/net/dpaa2/dpaa2_ethdev.c
>>> @@ -68,7 +68,17 @@
>>>  	dev_info->min_rx_bufsize = DPAA2_MIN_RX_BUF_SIZE;
>>>  	dev_info->max_rx_queues = (uint16_t)priv->nb_rx_queues;
>>>  	dev_info->max_tx_queues = (uint16_t)priv->nb_tx_queues;
>>> -
>>> +	dev_info->rx_offload_capa =
>>> +		DEV_RX_OFFLOAD_IPV4_CKSUM |
>>> +		DEV_RX_OFFLOAD_UDP_CKSUM |
>>> +		DEV_RX_OFFLOAD_TCP_CKSUM |
>>> +		DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM;
>>
>> Is there capabilities always enabled for all devices this driver
>> support? Or should these be read from some device registers?
>>
> Capabilities are always enabled for all devices (MC DPNI object in this 
> case)

Thanks for the clarification, is it same for the speed capabilities?

> 
>>> +	dev_info->tx_offload_capa =
>>> +		DEV_TX_OFFLOAD_IPV4_CKSUM |
>>> +		DEV_TX_OFFLOAD_UDP_CKSUM |
>>> +		DEV_TX_OFFLOAD_TCP_CKSUM |
>>> +		DEV_TX_OFFLOAD_SCTP_CKSUM |
>>> +		DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM;
>>>  	dev_info->speed_capa = ETH_LINK_SPEED_1G |
>>>  			ETH_LINK_SPEED_2_5G |
>>>  			ETH_LINK_SPEED_10G;
>> <...>
>>
> 
>
  

Patch

diff --git a/doc/guides/nics/features/dpaa2.ini b/doc/guides/nics/features/dpaa2.ini
index 20152a0..d50c62e 100644
--- a/doc/guides/nics/features/dpaa2.ini
+++ b/doc/guides/nics/features/dpaa2.ini
@@ -6,6 +6,8 @@ 
 [Features]
 Queue start/stop     = Y
 RSS hash             = Y
+L3 checksum offload  = Y
+L4 checksum offload  = Y
 Linux VFIO           = Y
 ARMv8                = Y
 Usage doc            = Y
diff --git a/drivers/bus/fslmc/portal/dpaa2_hw_pvt.h b/drivers/bus/fslmc/portal/dpaa2_hw_pvt.h
index 8efac2d..1af93a5 100644
--- a/drivers/bus/fslmc/portal/dpaa2_hw_pvt.h
+++ b/drivers/bus/fslmc/portal/dpaa2_hw_pvt.h
@@ -37,6 +37,12 @@ 
 #include <mc/fsl_mc_sys.h>
 #include <fsl_qbman_portal.h>
 
+#ifndef false
+#define false      0
+#endif
+#ifndef true
+#define true       1
+#endif
 #define DPAA2_DQRR_RING_SIZE	16
 	/** <Maximum number of slots available in RX ring*/
 
diff --git a/drivers/net/dpaa2/Makefile b/drivers/net/dpaa2/Makefile
index fadabb5..0e59203 100644
--- a/drivers/net/dpaa2/Makefile
+++ b/drivers/net/dpaa2/Makefile
@@ -66,6 +66,6 @@  DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_eal lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_mempool lib/librte_mbuf
 DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_common_dpaa2_qbman
 DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_bus_fslmc
-DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_pmd_dpaa2_pool
+DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_pool_dpaa2
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index 6de571a..2298246 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -68,7 +68,17 @@ 
 	dev_info->min_rx_bufsize = DPAA2_MIN_RX_BUF_SIZE;
 	dev_info->max_rx_queues = (uint16_t)priv->nb_rx_queues;
 	dev_info->max_tx_queues = (uint16_t)priv->nb_tx_queues;
-
+	dev_info->rx_offload_capa =
+		DEV_RX_OFFLOAD_IPV4_CKSUM |
+		DEV_RX_OFFLOAD_UDP_CKSUM |
+		DEV_RX_OFFLOAD_TCP_CKSUM |
+		DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM;
+	dev_info->tx_offload_capa =
+		DEV_TX_OFFLOAD_IPV4_CKSUM |
+		DEV_TX_OFFLOAD_UDP_CKSUM |
+		DEV_TX_OFFLOAD_TCP_CKSUM |
+		DEV_TX_OFFLOAD_SCTP_CKSUM |
+		DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM;
 	dev_info->speed_capa = ETH_LINK_SPEED_1G |
 			ETH_LINK_SPEED_2_5G |
 			ETH_LINK_SPEED_10G;
@@ -252,8 +262,13 @@ 
 	memset(&tx_conf_cfg, 0, sizeof(struct dpni_queue));
 	memset(&tx_flow_cfg, 0, sizeof(struct dpni_queue));
 
-	tc_id = 0;
-	flow_id = tx_queue_id;
+	if (priv->num_tc == 1) {
+		tc_id = 0;
+		flow_id = tx_queue_id % priv->num_dist_per_tc[tc_id];
+	} else {
+		tc_id = tx_queue_id;
+		flow_id = 0;
+	}
 
 	ret = dpni_set_queue(dpni, CMD_PRI_LOW, priv->token, DPNI_QUEUE_TX,
 			     tc_id, flow_id, options, &tx_flow_cfg);
@@ -302,6 +317,7 @@ 
 	struct dpaa2_dev_priv *priv = data->dev_private;
 	struct fsl_mc_io *dpni = (struct fsl_mc_io *)priv->hw;
 	struct dpni_queue cfg;
+	struct dpni_error_cfg	err_cfg;
 	uint16_t qdid;
 	struct dpni_queue_id qid;
 	struct dpaa2_queue *dpaa2_q;
@@ -337,6 +353,48 @@ 
 		dpaa2_q->fqid = qid.fqid;
 	}
 
+	ret = dpni_set_offload(dpni, CMD_PRI_LOW, priv->token,
+			       DPNI_OFF_RX_L3_CSUM, true);
+	if (ret) {
+		PMD_INIT_LOG(ERR, "Error to set RX l3 csum:Error = %d\n", ret);
+		return ret;
+	}
+
+	ret = dpni_set_offload(dpni, CMD_PRI_LOW, priv->token,
+			       DPNI_OFF_RX_L4_CSUM, true);
+	if (ret) {
+		PMD_INIT_LOG(ERR, "Error to get RX l4 csum:Error = %d\n", ret);
+		return ret;
+	}
+
+	ret = dpni_set_offload(dpni, CMD_PRI_LOW, priv->token,
+			       DPNI_OFF_TX_L3_CSUM, true);
+	if (ret) {
+		PMD_INIT_LOG(ERR, "Error to set TX l3 csum:Error = %d\n", ret);
+		return ret;
+	}
+
+	ret = dpni_set_offload(dpni, CMD_PRI_LOW, priv->token,
+			       DPNI_OFF_TX_L4_CSUM, true);
+	if (ret) {
+		PMD_INIT_LOG(ERR, "Error to get TX l4 csum:Error = %d\n", ret);
+		return ret;
+	}
+
+	/*checksum errors, send them to normal path and set it in annotation */
+	err_cfg.errors = DPNI_ERROR_L3CE | DPNI_ERROR_L4CE;
+
+	err_cfg.error_action = DPNI_ERROR_ACTION_CONTINUE;
+	err_cfg.set_frame_annotation = true;
+
+	ret = dpni_set_errors_behavior(dpni, CMD_PRI_LOW,
+				       priv->token, &err_cfg);
+	if (ret) {
+		PMD_INIT_LOG(ERR, "Error to dpni_set_errors_behavior:"
+			     "code = %d\n", ret);
+		return ret;
+	}
+
 	return 0;
 }
 
@@ -453,7 +511,13 @@ 
 	 */
 	priv->nb_rx_queues = priv->num_dist_per_tc[DPAA2_DEF_TC];
 
-	priv->nb_tx_queues = attr.num_queues;
+	if (attr.num_tcs == 1)
+		priv->nb_tx_queues = attr.num_queues;
+	else
+		priv->nb_tx_queues = attr.num_tcs;
+
+	PMD_INIT_LOG(DEBUG, "num_tc %d", priv->num_tc);
+	PMD_INIT_LOG(DEBUG, "nb_rx_queues %d", priv->nb_rx_queues);
 
 	eth_dev->data->nb_rx_queues = priv->nb_rx_queues;
 	eth_dev->data->nb_tx_queues = priv->nb_tx_queues;