[v2,3/4] net/bnxt: fix cleanup and check for ulp context alloc

Message ID 20200731172302.5292-4-ajit.khaparde@broadcom.com (mailing list archive)
State Superseded, archived
Delegated to: Ajit Khaparde
Headers
Series [v2,1/4] net/bnxt: fix loopback parif for egress flows |

Checks

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

Commit Message

Ajit Khaparde July 31, 2020, 5:23 p.m. UTC
  From: Somnath Kotur <somnath.kotur@broadcom.com>

Set ulp_ctx explicitly to NULL in ulp_ctx_deinit() so that representor
init is aborted if parent ulp context is not initialized.
Also check for the same before creation of port default rules.
Additional checks in VF rep dev ops for proper parent dev initialization

Fixes: 322bd6e70272 ("net/bnxt: add port representor infrastructure")
Fixes: 313ac35ac701 ("net/bnxt: support ULP session manager init")

Reviewed-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
---
 drivers/net/bnxt/bnxt_reps.c            | 18 ++++++++++++++++++
 drivers/net/bnxt/tf_ulp/bnxt_ulp.c      |  2 ++
 drivers/net/bnxt/tf_ulp/ulp_def_rules.c |  2 +-
 3 files changed, 21 insertions(+), 1 deletion(-)
  

Comments

Thomas Monjalon Aug. 5, 2020, 8:27 p.m. UTC | #1
31/07/2020 19:23, Ajit Khaparde:
> From: Somnath Kotur <somnath.kotur@broadcom.com>
> 
> Set ulp_ctx explicitly to NULL in ulp_ctx_deinit() so that representor
> init is aborted if parent ulp context is not initialized.
> Also check for the same before creation of port default rules.
> Additional checks in VF rep dev ops for proper parent dev initialization

Looks like it should be separate patche.

I don't know what is "ulp", but if it an acronym,
it should be in capital letters.

> Fixes: 322bd6e70272 ("net/bnxt: add port representor infrastructure")
> Fixes: 313ac35ac701 ("net/bnxt: support ULP session manager init")

Not clear it is a fix.
Looks like adding more checks.

> Reviewed-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>

Reviewed-by should be below Signed-off-by.

> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
  

Patch

diff --git a/drivers/net/bnxt/bnxt_reps.c b/drivers/net/bnxt/bnxt_reps.c
index 6fa9a30d2..2941aff7b 100644
--- a/drivers/net/bnxt/bnxt_reps.c
+++ b/drivers/net/bnxt/bnxt_reps.c
@@ -319,6 +319,7 @@  static int bnxt_vfr_alloc(struct rte_eth_dev *vfr_ethdev)
 {
 	int rc = 0;
 	struct bnxt_vf_representor *vfr = vfr_ethdev->data->dev_private;
+	struct bnxt *parent_bp;
 
 	if (!vfr || !vfr->parent_dev) {
 		PMD_DRV_LOG(ERR,
@@ -326,6 +327,13 @@  static int bnxt_vfr_alloc(struct rte_eth_dev *vfr_ethdev)
 		return -ENOMEM;
 	}
 
+	parent_bp = vfr->parent_dev->data->dev_private;
+	if (parent_bp && !parent_bp->ulp_ctx) {
+		PMD_DRV_LOG(ERR,
+			    "ulp context not allocated for parent\n");
+		return -EIO;
+	}
+
 	/* Check if representor has been already allocated in FW */
 	if (vfr->vfr_tx_cfa_action)
 		return 0;
@@ -534,6 +542,11 @@  int bnxt_vf_rep_rx_queue_setup_op(struct rte_eth_dev *eth_dev,
 		return -EINVAL;
 	}
 
+	if (!parent_bp->rx_queues) {
+		PMD_DRV_LOG(ERR, "Parent Rx qs not configured yet\n");
+		return -EINVAL;
+	}
+
 	parent_rxq = parent_bp->rx_queues[queue_idx];
 	if (!parent_rxq) {
 		PMD_DRV_LOG(ERR, "Parent RxQ has not been configured yet\n");
@@ -628,6 +641,11 @@  int bnxt_vf_rep_tx_queue_setup_op(struct rte_eth_dev *eth_dev,
 		return -EINVAL;
 	}
 
+	if (!parent_bp->tx_queues) {
+		PMD_DRV_LOG(ERR, "Parent Tx qs not configured yet\n");
+		return -EINVAL;
+	}
+
 	parent_txq = parent_bp->tx_queues[queue_idx];
 	if (!parent_txq) {
 		PMD_DRV_LOG(ERR, "Parent TxQ has not been configured yet\n");
diff --git a/drivers/net/bnxt/tf_ulp/bnxt_ulp.c b/drivers/net/bnxt/tf_ulp/bnxt_ulp.c
index 077527f78..c19cd1d21 100644
--- a/drivers/net/bnxt/tf_ulp/bnxt_ulp.c
+++ b/drivers/net/bnxt/tf_ulp/bnxt_ulp.c
@@ -884,6 +884,8 @@  bnxt_ulp_deinit(struct bnxt *bp)
 	ulp_session_deinit(session);
 
 	rte_free(bp->ulp_ctx);
+
+	bp->ulp_ctx = NULL;
 }
 
 /* Function to set the Mark DB into the context */
diff --git a/drivers/net/bnxt/tf_ulp/ulp_def_rules.c b/drivers/net/bnxt/tf_ulp/ulp_def_rules.c
index 9fb1a028f..46acc1d65 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_def_rules.c
+++ b/drivers/net/bnxt/tf_ulp/ulp_def_rules.c
@@ -465,7 +465,7 @@  bnxt_ulp_create_df_rules(struct bnxt *bp)
 	int rc;
 
 	if (!BNXT_TRUFLOW_EN(bp) ||
-	    BNXT_ETH_DEV_IS_REPRESENTOR(bp->eth_dev))
+	    BNXT_ETH_DEV_IS_REPRESENTOR(bp->eth_dev) || !bp->ulp_ctx)
 		return 0;
 
 	port_id = bp->eth_dev->data->port_id;