DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev]  [PATCH] net/octeontx2: fix driver reconfiguration
@ 2019-07-22 13:05 kkanas
  2019-07-22 14:39 ` Jerin Jacob Kollanukkaran
  0 siblings, 1 reply; 4+ messages in thread
From: kkanas @ 2019-07-22 13:05 UTC (permalink / raw)
  To: dev, Jerin Jacob, Nithin Dabilpuram, Kiran Kumar K; +Cc: Krzysztof Kanas

From: Krzysztof Kanas <kkanas@marvell.com>

When configure returns error, e.g. in case not supported offloads (outer ip
and sctp) driver released Rx,Tx queues. Then in case of correct
configuration the driver could not start due to queues already released but
the driver thought it was configured correctly.

Secondly if driver returns error from configuration librte_ethdev will
release, rx queues and tx queues, without chaining driver configured state.

Fix that by 'releasing' configuration and changing driver state when error
is returned from otx2_nix_configure.

Fixes: 548b5839a32b ("net/octeontx2: add device configure operation")

Signed-off-by: Krzysztof Kanas <kkanas@marvell.com>
Reviewed-by: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
Reviewed-by: Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>
---
 drivers/net/octeontx2/otx2_ethdev.c | 65 +++++++++++++++++------------
 1 file changed, 38 insertions(+), 27 deletions(-)

diff --git a/drivers/net/octeontx2/otx2_ethdev.c b/drivers/net/octeontx2/otx2_ethdev.c
index fcb1869d5871..e6f81c4c9fec 100644
--- a/drivers/net/octeontx2/otx2_ethdev.c
+++ b/drivers/net/octeontx2/otx2_ethdev.c
@@ -1185,38 +1185,46 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
 	/* Sanity checks */
 	if (rte_eal_has_hugepages() == 0) {
 		otx2_err("Huge page is not configured");
-		goto fail;
+		goto fail_configure;
 	}
 
 	if (rte_eal_iova_mode() != RTE_IOVA_VA) {
 		otx2_err("iova mode should be va");
-		goto fail;
+		goto fail_configure;
 	}
 
 	if (conf->link_speeds & ETH_LINK_SPEED_FIXED) {
 		otx2_err("Setting link speed/duplex not supported");
-		goto fail;
+		goto fail_configure;
 	}
 
 	if (conf->dcb_capability_en == 1) {
 		otx2_err("dcb enable is not supported");
-		goto fail;
+		goto fail_configure;
 	}
 
 	if (conf->fdir_conf.mode != RTE_FDIR_MODE_NONE) {
 		otx2_err("Flow director is not supported");
-		goto fail;
+		goto fail_configure;
 	}
 
 	if (rxmode->mq_mode != ETH_MQ_RX_NONE &&
 	    rxmode->mq_mode != ETH_MQ_RX_RSS) {
 		otx2_err("Unsupported mq rx mode %d", rxmode->mq_mode);
-		goto fail;
+		goto fail_configure;
 	}
 
 	if (txmode->mq_mode != ETH_MQ_TX_NONE) {
 		otx2_err("Unsupported mq tx mode %d", txmode->mq_mode);
-		goto fail;
+		goto fail_configure;
+	}
+
+	if (otx2_dev_is_A0(dev) &&
+	    (txmode->offloads & DEV_TX_OFFLOAD_SCTP_CKSUM) &&
+	    ((txmode->offloads & DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM) ||
+	    (txmode->offloads & DEV_TX_OFFLOAD_OUTER_UDP_CKSUM))) {
+		otx2_err("Outer IP and SCTP checksum unsupported");
+		goto fail_configure;
 	}
 
 	/* Free the resources allocated from the previous configure */
@@ -1230,20 +1238,11 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
 		nix_set_nop_rxtx_function(eth_dev);
 		rc = nix_store_queue_cfg_and_then_release(eth_dev);
 		if (rc)
-			goto fail;
+			goto fail_configure;
 		otx2_nix_tm_fini(eth_dev);
 		nix_lf_free(dev);
 	}
 
-	if (otx2_dev_is_Ax(dev) &&
-	    (txmode->offloads & DEV_TX_OFFLOAD_SCTP_CKSUM) &&
-	    ((txmode->offloads & DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM) ||
-	    (txmode->offloads & DEV_TX_OFFLOAD_OUTER_UDP_CKSUM))) {
-		otx2_err("Outer IP and SCTP checksum unsupported");
-		rc = -EINVAL;
-		goto fail;
-	}
-
 	dev->rx_offloads = rxmode->offloads;
 	dev->tx_offloads = txmode->offloads;
 	dev->rx_offload_flags |= nix_rx_offload_flags(eth_dev);
@@ -1257,7 +1256,7 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
 	rc = nix_lf_alloc(dev, nb_rxq, nb_txq);
 	if (rc) {
 		otx2_err("Failed to init nix_lf rc=%d", rc);
-		goto fail;
+		goto fail_offloads;
 	}
 
 	/* Configure RSS */
@@ -1277,14 +1276,14 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
 	rc = otx2_nix_vlan_offload_init(eth_dev);
 	if (rc) {
 		otx2_err("Failed to init vlan offload rc=%d", rc);
-		goto free_nix_lf;
+		goto tm_fini;
 	}
 
 	/* Register queue IRQs */
 	rc = oxt2_nix_register_queue_irqs(eth_dev);
 	if (rc) {
 		otx2_err("Failed to register queue interrupts rc=%d", rc);
-		goto free_nix_lf;
+		goto vlan_fini;
 	}
 
 	/* Register cq IRQs */
@@ -1292,7 +1291,7 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
 		if (eth_dev->data->nb_rx_queues > dev->cints) {
 			otx2_err("Rx interrupt cannot be enabled, rxq > %d",
 				 dev->cints);
-			goto free_nix_lf;
+			goto q_irq_fini;
 		}
 		/* Rx interrupt feature cannot work with vector mode because,
 		 * vector mode doesn't process packets unless min 4 pkts are
@@ -1304,7 +1303,7 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
 		rc = oxt2_nix_register_cq_irqs(eth_dev);
 		if (rc) {
 			otx2_err("Failed to register CQ interrupts rc=%d", rc);
-			goto free_nix_lf;
+			goto q_irq_fini;
 		}
 	}
 
@@ -1312,13 +1311,13 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
 	rc = cgx_intlbk_enable(dev, eth_dev->data->dev_conf.lpbk_mode);
 	if (rc) {
 		otx2_err("Failed to configure cgx loop back mode rc=%d", rc);
-		goto free_nix_lf;
+		goto q_irq_fini;
 	}
 
 	rc = otx2_nix_rxchan_bpid_cfg(eth_dev, true);
 	if (rc) {
 		otx2_err("Failed to configure nix rx chan bpid cfg rc=%d", rc);
-		goto free_nix_lf;
+		goto q_irq_fini;
 	}
 
 	/* Enable PTP if it was requested by the app or if it is already
@@ -1338,7 +1337,7 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
 	if (dev->configured == 1) {
 		rc = nix_restore_queue_cfg(eth_dev);
 		if (rc)
-			goto free_nix_lf;
+			goto cq_fini;
 	}
 
 	/* Update the mac address */
@@ -1362,9 +1361,21 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
 	dev->configured_nb_tx_qs = data->nb_tx_queues;
 	return 0;
 
+cq_fini:
+	oxt2_nix_unregister_cq_irqs(eth_dev);
+q_irq_fini:
+	oxt2_nix_unregister_queue_irqs(eth_dev);
+vlan_fini:
+	otx2_nix_vlan_fini(eth_dev);
+tm_fini:
+	otx2_nix_tm_fini(eth_dev);
 free_nix_lf:
-	rc = nix_lf_free(dev);
-fail:
+	nix_lf_free(dev);
+fail_offloads:
+	dev->rx_offload_flags &= ~nix_rx_offload_flags(eth_dev);
+	dev->tx_offload_flags &= ~nix_tx_offload_flags(eth_dev);
+fail_configure:
+	dev->configured = 0;
 	return rc;
 }
 
-- 
2.21.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] net/octeontx2: fix driver reconfiguration
  2019-07-22 13:05 [dpdk-dev] [PATCH] net/octeontx2: fix driver reconfiguration kkanas
@ 2019-07-22 14:39 ` Jerin Jacob Kollanukkaran
  2019-07-22 14:58   ` [dpdk-dev] [PATCH v2] " kkanas
  0 siblings, 1 reply; 4+ messages in thread
From: Jerin Jacob Kollanukkaran @ 2019-07-22 14:39 UTC (permalink / raw)
  To: Krzysztof Kanas, dev, Nithin Kumar Dabilpuram, Kiran Kumar Kokkilagadda
  Cc: Krzysztof Kanas

> -----Original Message-----
> From: kkanas@marvell.com <kkanas@marvell.com>
> Sent: Monday, July 22, 2019 6:36 PM
> To: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Nithin
> Kumar Dabilpuram <ndabilpuram@marvell.com>; Kiran Kumar Kokkilagadda
> <kirankumark@marvell.com>
> Cc: Krzysztof Kanas <kkanas@marvell.com>
> Subject: [dpdk-dev] [PATCH] net/octeontx2: fix driver reconfiguration
> 
> From: Krzysztof Kanas <kkanas@marvell.com>
> 
> When configure returns error, e.g. in case not supported offloads (outer ip
> and sctp) driver released Rx,Tx queues. Then in case of correct configuration
> the driver could not start due to queues already released but the driver
> thought it was configured correctly.
> 
> Secondly if driver returns error from configuration librte_ethdev will release,
> rx queues and tx queues, without chaining driver configured state.
> 
> Fix that by 'releasing' configuration and changing driver state when error is
> returned from otx2_nix_configure.
> 
> Fixes: 548b5839a32b ("net/octeontx2: add device configure operation")
> 
> Signed-off-by: Krzysztof Kanas <kkanas@marvell.com>
> Reviewed-by: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Reviewed-by: Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>
> ---
>  drivers/net/octeontx2/otx2_ethdev.c | 65 +++++++++++++++++------------
>  1 file changed, 38 insertions(+), 27 deletions(-)
> 
> +	if (otx2_dev_is_A0(dev) &&

Please change to otx2_dev_is_Ax(). See below.

> +	    (txmode->offloads & DEV_TX_OFFLOAD_SCTP_CKSUM) &&
> +	    ((txmode->offloads & DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM)
> ||
> +	    (txmode->offloads & DEV_TX_OFFLOAD_OUTER_UDP_CKSUM))) {
> +		otx2_err("Outer IP and SCTP checksum unsupported");
> +		goto fail_configure;
>  	}
> 
>  	/* Free the resources allocated from the previous configure */ @@ -
> 1230,20 +1238,11 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
>  		nix_set_nop_rxtx_function(eth_dev);
>  		rc = nix_store_queue_cfg_and_then_release(eth_dev);
>  		if (rc)
> -			goto fail;
> +			goto fail_configure;
>  		otx2_nix_tm_fini(eth_dev);
>  		nix_lf_free(dev);
>  	}
> 
> -	if (otx2_dev_is_Ax(dev) &&

See above.

> -	    (txmode->offloads & DEV_TX_OFFLOAD_SCTP_CKSUM) &&
> -	    ((txmode->offloads & DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM)
> ||
> -	    (txmode->offloads & DEV_TX_OFFLOAD_OUTER_UDP_CKSUM))) {
> -		otx2_err("Outer IP and SCTP checksum unsupported");
> -		rc = -EINVAL;
> -		goto fail;
> -	}

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [dpdk-dev]  [PATCH v2] net/octeontx2: fix driver reconfiguration
  2019-07-22 14:39 ` Jerin Jacob Kollanukkaran
@ 2019-07-22 14:58   ` kkanas
  2019-07-22 16:02     ` Jerin Jacob Kollanukkaran
  0 siblings, 1 reply; 4+ messages in thread
From: kkanas @ 2019-07-22 14:58 UTC (permalink / raw)
  To: dev, Jerin Jacob, Nithin Dabilpuram, Kiran Kumar K; +Cc: Krzysztof Kanas

From: Krzysztof Kanas <kkanas@marvell.com>

When configure returns error, e.g. in case not supported offloads (outer ip
and sctp) driver released Rx,Tx queues. Then in case of correct
configuration the driver could not start due to queues already released but
the driver thought it was configured correctly.

Secondly if driver returns error from configuration librte_ethdev will
release, rx queues and tx queues, without chaining driver configured state.

Fix that by 'releasing' configuration and changing driver state when error
is returned from otx2_nix_configure.

Fixes: 548b5839a32b ("net/octeontx2: add device configure operation")

Signed-off-by: Krzysztof Kanas <kkanas@marvell.com>
Reviewed-by: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
Reviewed-by: Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>
---
 drivers/net/octeontx2/otx2_ethdev.c | 65 +++++++++++++++++------------
 1 file changed, 38 insertions(+), 27 deletions(-)

diff --git a/drivers/net/octeontx2/otx2_ethdev.c b/drivers/net/octeontx2/otx2_ethdev.c
index fcb1869d5871..c6e0a515f0c4 100644
--- a/drivers/net/octeontx2/otx2_ethdev.c
+++ b/drivers/net/octeontx2/otx2_ethdev.c
@@ -1185,38 +1185,46 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
 	/* Sanity checks */
 	if (rte_eal_has_hugepages() == 0) {
 		otx2_err("Huge page is not configured");
-		goto fail;
+		goto fail_configure;
 	}
 
 	if (rte_eal_iova_mode() != RTE_IOVA_VA) {
 		otx2_err("iova mode should be va");
-		goto fail;
+		goto fail_configure;
 	}
 
 	if (conf->link_speeds & ETH_LINK_SPEED_FIXED) {
 		otx2_err("Setting link speed/duplex not supported");
-		goto fail;
+		goto fail_configure;
 	}
 
 	if (conf->dcb_capability_en == 1) {
 		otx2_err("dcb enable is not supported");
-		goto fail;
+		goto fail_configure;
 	}
 
 	if (conf->fdir_conf.mode != RTE_FDIR_MODE_NONE) {
 		otx2_err("Flow director is not supported");
-		goto fail;
+		goto fail_configure;
 	}
 
 	if (rxmode->mq_mode != ETH_MQ_RX_NONE &&
 	    rxmode->mq_mode != ETH_MQ_RX_RSS) {
 		otx2_err("Unsupported mq rx mode %d", rxmode->mq_mode);
-		goto fail;
+		goto fail_configure;
 	}
 
 	if (txmode->mq_mode != ETH_MQ_TX_NONE) {
 		otx2_err("Unsupported mq tx mode %d", txmode->mq_mode);
-		goto fail;
+		goto fail_configure;
+	}
+
+	if (otx2_dev_is_Ax(dev) &&
+	    (txmode->offloads & DEV_TX_OFFLOAD_SCTP_CKSUM) &&
+	    ((txmode->offloads & DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM) ||
+	    (txmode->offloads & DEV_TX_OFFLOAD_OUTER_UDP_CKSUM))) {
+		otx2_err("Outer IP and SCTP checksum unsupported");
+		goto fail_configure;
 	}
 
 	/* Free the resources allocated from the previous configure */
@@ -1230,20 +1238,11 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
 		nix_set_nop_rxtx_function(eth_dev);
 		rc = nix_store_queue_cfg_and_then_release(eth_dev);
 		if (rc)
-			goto fail;
+			goto fail_configure;
 		otx2_nix_tm_fini(eth_dev);
 		nix_lf_free(dev);
 	}
 
-	if (otx2_dev_is_Ax(dev) &&
-	    (txmode->offloads & DEV_TX_OFFLOAD_SCTP_CKSUM) &&
-	    ((txmode->offloads & DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM) ||
-	    (txmode->offloads & DEV_TX_OFFLOAD_OUTER_UDP_CKSUM))) {
-		otx2_err("Outer IP and SCTP checksum unsupported");
-		rc = -EINVAL;
-		goto fail;
-	}
-
 	dev->rx_offloads = rxmode->offloads;
 	dev->tx_offloads = txmode->offloads;
 	dev->rx_offload_flags |= nix_rx_offload_flags(eth_dev);
@@ -1257,7 +1256,7 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
 	rc = nix_lf_alloc(dev, nb_rxq, nb_txq);
 	if (rc) {
 		otx2_err("Failed to init nix_lf rc=%d", rc);
-		goto fail;
+		goto fail_offloads;
 	}
 
 	/* Configure RSS */
@@ -1277,14 +1276,14 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
 	rc = otx2_nix_vlan_offload_init(eth_dev);
 	if (rc) {
 		otx2_err("Failed to init vlan offload rc=%d", rc);
-		goto free_nix_lf;
+		goto tm_fini;
 	}
 
 	/* Register queue IRQs */
 	rc = oxt2_nix_register_queue_irqs(eth_dev);
 	if (rc) {
 		otx2_err("Failed to register queue interrupts rc=%d", rc);
-		goto free_nix_lf;
+		goto vlan_fini;
 	}
 
 	/* Register cq IRQs */
@@ -1292,7 +1291,7 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
 		if (eth_dev->data->nb_rx_queues > dev->cints) {
 			otx2_err("Rx interrupt cannot be enabled, rxq > %d",
 				 dev->cints);
-			goto free_nix_lf;
+			goto q_irq_fini;
 		}
 		/* Rx interrupt feature cannot work with vector mode because,
 		 * vector mode doesn't process packets unless min 4 pkts are
@@ -1304,7 +1303,7 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
 		rc = oxt2_nix_register_cq_irqs(eth_dev);
 		if (rc) {
 			otx2_err("Failed to register CQ interrupts rc=%d", rc);
-			goto free_nix_lf;
+			goto q_irq_fini;
 		}
 	}
 
@@ -1312,13 +1311,13 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
 	rc = cgx_intlbk_enable(dev, eth_dev->data->dev_conf.lpbk_mode);
 	if (rc) {
 		otx2_err("Failed to configure cgx loop back mode rc=%d", rc);
-		goto free_nix_lf;
+		goto q_irq_fini;
 	}
 
 	rc = otx2_nix_rxchan_bpid_cfg(eth_dev, true);
 	if (rc) {
 		otx2_err("Failed to configure nix rx chan bpid cfg rc=%d", rc);
-		goto free_nix_lf;
+		goto q_irq_fini;
 	}
 
 	/* Enable PTP if it was requested by the app or if it is already
@@ -1338,7 +1337,7 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
 	if (dev->configured == 1) {
 		rc = nix_restore_queue_cfg(eth_dev);
 		if (rc)
-			goto free_nix_lf;
+			goto cq_fini;
 	}
 
 	/* Update the mac address */
@@ -1362,9 +1361,21 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
 	dev->configured_nb_tx_qs = data->nb_tx_queues;
 	return 0;
 
+cq_fini:
+	oxt2_nix_unregister_cq_irqs(eth_dev);
+q_irq_fini:
+	oxt2_nix_unregister_queue_irqs(eth_dev);
+vlan_fini:
+	otx2_nix_vlan_fini(eth_dev);
+tm_fini:
+	otx2_nix_tm_fini(eth_dev);
 free_nix_lf:
-	rc = nix_lf_free(dev);
-fail:
+	nix_lf_free(dev);
+fail_offloads:
+	dev->rx_offload_flags &= ~nix_rx_offload_flags(eth_dev);
+	dev->tx_offload_flags &= ~nix_tx_offload_flags(eth_dev);
+fail_configure:
+	dev->configured = 0;
 	return rc;
 }
 
-- 
2.21.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH v2] net/octeontx2: fix driver reconfiguration
  2019-07-22 14:58   ` [dpdk-dev] [PATCH v2] " kkanas
@ 2019-07-22 16:02     ` Jerin Jacob Kollanukkaran
  0 siblings, 0 replies; 4+ messages in thread
From: Jerin Jacob Kollanukkaran @ 2019-07-22 16:02 UTC (permalink / raw)
  To: Krzysztof Kanas, dev, Nithin Kumar Dabilpuram, Kiran Kumar Kokkilagadda
  Cc: Krzysztof Kanas, Ferruh Yigit

> -----Original Message-----
> From: kkanas@marvell.com <kkanas@marvell.com>
> Sent: Monday, July 22, 2019 8:29 PM
> To: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Nithin
> Kumar Dabilpuram <ndabilpuram@marvell.com>; Kiran Kumar Kokkilagadda
> <kirankumark@marvell.com>
> Cc: Krzysztof Kanas <kkanas@marvell.com>
> Subject: [dpdk-dev] [PATCH v2] net/octeontx2: fix driver reconfiguration
> 
> From: Krzysztof Kanas <kkanas@marvell.com>
> 
> When configure returns error, e.g. in case not supported offloads (outer ip
> and sctp) driver released Rx,Tx queues. Then in case of correct configuration
> the driver could not start due to queues already released but the driver
> thought it was configured correctly.
> 
> Secondly if driver returns error from configuration librte_ethdev will release,
> rx queues and tx queues, without chaining driver configured state.
> 
> Fix that by 'releasing' configuration and changing driver state when error is
> returned from otx2_nix_configure.
> 
> Fixes: 548b5839a32b ("net/octeontx2: add device configure operation")
> 
> Signed-off-by: Krzysztof Kanas <kkanas@marvell.com>
> Reviewed-by: Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>

Acked-by: Jerin Jacob <jerinj@marvell.com>

Applied to dpdk-next-net-mrvl/master. Thanks

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-07-22 16:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-22 13:05 [dpdk-dev] [PATCH] net/octeontx2: fix driver reconfiguration kkanas
2019-07-22 14:39 ` Jerin Jacob Kollanukkaran
2019-07-22 14:58   ` [dpdk-dev] [PATCH v2] " kkanas
2019-07-22 16:02     ` Jerin Jacob Kollanukkaran

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).