Skip to contentSkip to conversation

Error report improvement and fix

PATCH New Gavin Li 2 participants 17 messages Sep 6, 2024 – Sep 25, 2024 Network drivers

lore
Share & tools
lore

Gavin Li

author
#0 lore raw

cover Error report improvement and fix

This patch set is to improve error handling in pmd and under layer.

Gavin Li (3):
net/mlx5: set rte errno if malloc failed net/mlx5/hws: add log for failing to create rule in HWS net/mlx5/hws: print CQE error syndrome and more information

 drivers/net/mlx5/hws/mlx5dr_rule.c |  6 ++++++
 drivers/net/mlx5/hws/mlx5dr_send.c |  9 ++++++++-
 drivers/net/mlx5/mlx5_flow_hw.c    | 31 +++++++++++++++++++++++-------
 3 files changed, 38 insertions(+), 8 deletions(-)
Signature · 1 line
2.34.1
author
#1 lore raw

PATCH 1/3 net/mlx5: set rte errno if malloc failed

From: "Minggang Li (Gavin)" <[email protected]>

rte_errno should be set if anything wrong happened in under layer so that user can figure out what's going on.

There were some cases that did not set it when ipool allcation failed. To fix the issue, set rte_errno to ENOMEM if mlx5_ipool_malloc failed to allocate ID.

Fixes c40c061a02 ("net/mlx5: add basic flow queue operation")
Fixes 48fbb0e93d ("net/mlx5: support flow meter mark indirect action with HWS")
---
 drivers/net/mlx5/mlx5_flow_hw.c | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)
drivers/net/mlx5/mlx5_flow_hw.c +24 -7
index 50888944a5..509de2a6a4 100644
@@ -1897,7 +1897,7 @@ flow_hw_meter_mark_alloc(struct rte_eth_dev *dev, uint32_t queue,
 	const struct rte_flow_action_meter_mark *meter_mark = action->conf;
 	struct mlx5_aso_mtr *aso_mtr;
 	struct mlx5_flow_meter_info *fm;
-	uint32_t mtr_id;
+	uint32_t mtr_id = 0;
 	uintptr_t handle = (uintptr_t)MLX5_INDIRECT_ACTION_TYPE_METER_MARK <<
 					MLX5_INDIRECT_ACTION_TYPE_OFFSET;
 
@@ -1909,8 +1909,15 @@ flow_hw_meter_mark_alloc(struct rte_eth_dev *dev, uint32_t queue,
 	if (meter_mark->profile == NULL)
 		return NULL;
 	aso_mtr = mlx5_ipool_malloc(pool->idx_pool, &mtr_id);
-	if (!aso_mtr)
+	if (!aso_mtr) {
+		rte_flow_error_set(error, ENOMEM,
+				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				   NULL,
+				   "failed to allocate aso meter entry");
+		if (mtr_id)
+			mlx5_ipool_free(pool->idx_pool, mtr_id);
 		return NULL;
+	}
 	/* Fill the flow meter parameters. */
 	aso_mtr->type = ASO_METER_INDIRECT;
 	fm = &aso_mtr->fm;
@@ -3918,8 +3925,10 @@ flow_hw_async_flow_create(struct rte_eth_dev *dev,
 			return NULL;
 	}
 	flow = mlx5_ipool_malloc(table->flow, &flow_idx);
-	if (!flow)
+	if (!flow) {
+		rte_errno = ENOMEM;
 		goto error;
+	}
 	rule_acts = flow_hw_get_dr_action_buffer(priv, table, action_template_index, queue);
 	/*
 	 * Set the table here in order to know the destination table
@@ -3930,8 +3939,10 @@ flow_hw_async_flow_create(struct rte_eth_dev *dev,
 	flow->idx = flow_idx;
 	if (table->resource) {
 		mlx5_ipool_malloc(table->resource, &res_idx);
-		if (!res_idx)
+		if (!res_idx) {
+			rte_errno = ENOMEM;
 			goto error;
+		}
 		flow->res_idx = res_idx;
 	} else {
 		flow->res_idx = flow_idx;
@@ -4062,8 +4073,10 @@ flow_hw_async_flow_create_by_index(struct rte_eth_dev *dev,
 			return NULL;
 	}
 	flow = mlx5_ipool_malloc(table->flow, &flow_idx);
-	if (!flow)
+	if (!flow) {
+		rte_errno = ENOMEM;
 		goto error;
+	}
 	rule_acts = flow_hw_get_dr_action_buffer(priv, table, action_template_index, queue);
 	/*
 	 * Set the table here in order to know the destination table
@@ -4074,8 +4087,10 @@ flow_hw_async_flow_create_by_index(struct rte_eth_dev *dev,
 	flow->idx = flow_idx;
 	if (table->resource) {
 		mlx5_ipool_malloc(table->resource, &res_idx);
-		if (!res_idx)
+		if (!res_idx) {
+			rte_errno = ENOMEM;
 			goto error;
+		}
 		flow->res_idx = res_idx;
 	} else {
 		flow->res_idx = flow_idx;
@@ -4210,8 +4225,10 @@ flow_hw_async_flow_update(struct rte_eth_dev *dev,
 	nf->idx = of->idx;
 	if (table->resource) {
 		mlx5_ipool_malloc(table->resource, &res_idx);
-		if (!res_idx)
+		if (!res_idx) {
+			rte_errno = ENOMEM;
 			goto error;
+		}
 		nf->res_idx = res_idx;
 	} else {
 		nf->res_idx = of->res_idx;
Signature · 1 line
2.34.1
author
#2 lore raw

PATCH 2/3 net/mlx5/hws: add log for failing to create rule in HWS

In reply to Gavin Li

From: "Minggang Li (Gavin)" <[email protected]>

---
 drivers/net/mlx5/hws/mlx5dr_rule.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/net/mlx5/hws/mlx5dr_rule.c +6 -0
index 1edb7eac74..5d66d81ea5 100644
@@ -638,6 +638,7 @@ static int mlx5dr_rule_destroy_hws(struct mlx5dr_rule *rule,
 
 	/* Rule is not completed yet */
 	if (rule->status == MLX5DR_RULE_STATUS_CREATING) {
+		DR_LOG(NOTICE, "Cannot destroy, rule creation still in progress");
 		rte_errno = EBUSY;
 		return rte_errno;
 	}
@@ -806,12 +807,14 @@ static int mlx5dr_rule_enqueue_precheck(struct mlx5dr_rule *rule,
 	struct mlx5dr_context *ctx = rule->matcher->tbl->ctx;
 
 	if (unlikely(!attr->user_data)) {
+		DR_LOG(DEBUG, "User data must be provided for rule operations");
 		rte_errno = EINVAL;
 		return rte_errno;
 	}
 
 	/* Check if there is room in queue */
 	if (unlikely(mlx5dr_send_engine_full(&ctx->send_queue[attr->queue_id]))) {
+		DR_LOG(NOTICE, "No room in queue[%d]", attr->queue_id);
 		rte_errno = EBUSY;
 		return rte_errno;
 	}
@@ -823,6 +826,7 @@ static int mlx5dr_rule_enqueue_precheck_move(struct mlx5dr_rule *rule,
 					     struct mlx5dr_rule_attr *attr)
 {
 	if (unlikely(rule->status != MLX5DR_RULE_STATUS_CREATED)) {
+		DR_LOG(DEBUG, "Cannot move, rule status is invalid");
 		rte_errno = EINVAL;
 		return rte_errno;
 	}
@@ -835,6 +839,7 @@ static int mlx5dr_rule_enqueue_precheck_create(struct mlx5dr_rule *rule,
 {
 	if (unlikely(mlx5dr_matcher_is_in_resize(rule->matcher))) {
 		/* Matcher in resize - new rules are not allowed */
+		DR_LOG(NOTICE, "Resizing in progress, cannot create rule");
 		rte_errno = EAGAIN;
 		return rte_errno;
 	}
@@ -1068,6 +1073,7 @@ int mlx5dr_rule_hash_calculate(struct mlx5dr_matcher *matcher,
 	    mlx5dr_table_is_root(matcher->tbl) ||
 	    matcher->tbl->ctx->caps->access_index_mode == MLX5DR_MATCHER_INSERT_BY_HASH ||
 	    matcher->tbl->ctx->caps->flow_table_hash_type != MLX5_FLOW_TABLE_HASH_TYPE_CRC32) {
+		DR_LOG(DEBUG, "Matcher is not supported");
 		rte_errno = ENOTSUP;
 		return -rte_errno;
 	}
Signature · 1 line
2.34.1
author
#3 lore raw

PATCH 3/3 net/mlx5/hws: print CQE error syndrome and more information

In reply to Gavin Li

From: "Minggang Li (Gavin)" <[email protected]>

---
 drivers/net/mlx5/hws/mlx5dr_send.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/net/mlx5/hws/mlx5dr_send.c +8 -1
index 3022c50260..c931896a79 100644
@@ -598,8 +598,15 @@ static void mlx5dr_send_engine_poll_cq(struct mlx5dr_send_engine *queue,
 	    cqe_owner != sw_own)
 		return;
 
-	if (unlikely(cqe_opcode != MLX5_CQE_REQ))
+	if (unlikely(cqe_opcode != MLX5_CQE_REQ)) {
+		struct mlx5_err_cqe *err_cqe = (struct mlx5_err_cqe *)cqe;
+
+		DR_LOG(ERR, "CQE ERR:0x%x, Vender_ERR:0x%x, OP:0x%x, QPN:0x%x, WQE_CNT:0x%x",
+			err_cqe->syndrome, err_cqe->vendor_err_synd, cqe_opcode,
+			(rte_be_to_cpu_32(err_cqe->s_wqe_opcode_qpn) & 0xffffff),
+			rte_be_to_cpu_16(err_cqe->wqe_counter));
 		queue->err = true;
+	}
 
 	rte_io_rmb();
 
Signature · 1 line
2.34.1
author
#4 lore raw

cover Error report improvement and fix

This patch set is to improve error handling in pmd and under layer.

Gavin Li (3):
net/mlx5: set rte errno if malloc failed
---
changelog:
v0->v1
- Fix typo in commit message
---
net/mlx5/hws: add log for failing to create rule in HWS net/mlx5/hws: print CQE error syndrome and more information

 drivers/net/mlx5/hws/mlx5dr_rule.c |  6 ++++++
 drivers/net/mlx5/hws/mlx5dr_send.c |  9 ++++++++-
 drivers/net/mlx5/mlx5_flow_hw.c    | 31 +++++++++++++++++++++++-------
 3 files changed, 38 insertions(+), 8 deletions(-)
Signature · 1 line
2.34.1
author
#5 lore raw

PATCH 1/3 net/mlx5: set rte errno if malloc failed

From: "Minggang Li (Gavin)" <[email protected]>

rte_errno should be set if anything wrong happened in under layer so that user can figure out what's going on.

There were some cases that did not set it when ipool allocation failed. To fix the issue, set rte_errno to ENOMEM if mlx5_ipool_malloc failed to allocate ID.

Fixes c40c061a02 ("net/mlx5: add basic flow queue operation")
Fixes 48fbb0e93d ("net/mlx5: support flow meter mark indirect action with HWS")
---
 drivers/net/mlx5/mlx5_flow_hw.c | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)
drivers/net/mlx5/mlx5_flow_hw.c +24 -7
index 50888944a5..509de2a6a4 100644
@@ -1897,7 +1897,7 @@ flow_hw_meter_mark_alloc(struct rte_eth_dev *dev, uint32_t queue,
 	const struct rte_flow_action_meter_mark *meter_mark = action->conf;
 	struct mlx5_aso_mtr *aso_mtr;
 	struct mlx5_flow_meter_info *fm;
-	uint32_t mtr_id;
+	uint32_t mtr_id = 0;
 	uintptr_t handle = (uintptr_t)MLX5_INDIRECT_ACTION_TYPE_METER_MARK <<
 					MLX5_INDIRECT_ACTION_TYPE_OFFSET;
 
@@ -1909,8 +1909,15 @@ flow_hw_meter_mark_alloc(struct rte_eth_dev *dev, uint32_t queue,
 	if (meter_mark->profile == NULL)
 		return NULL;
 	aso_mtr = mlx5_ipool_malloc(pool->idx_pool, &mtr_id);
-	if (!aso_mtr)
+	if (!aso_mtr) {
+		rte_flow_error_set(error, ENOMEM,
+				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				   NULL,
+				   "failed to allocate aso meter entry");
+		if (mtr_id)
+			mlx5_ipool_free(pool->idx_pool, mtr_id);
 		return NULL;
+	}
 	/* Fill the flow meter parameters. */
 	aso_mtr->type = ASO_METER_INDIRECT;
 	fm = &aso_mtr->fm;
@@ -3918,8 +3925,10 @@ flow_hw_async_flow_create(struct rte_eth_dev *dev,
 			return NULL;
 	}
 	flow = mlx5_ipool_malloc(table->flow, &flow_idx);
-	if (!flow)
+	if (!flow) {
+		rte_errno = ENOMEM;
 		goto error;
+	}
 	rule_acts = flow_hw_get_dr_action_buffer(priv, table, action_template_index, queue);
 	/*
 	 * Set the table here in order to know the destination table
@@ -3930,8 +3939,10 @@ flow_hw_async_flow_create(struct rte_eth_dev *dev,
 	flow->idx = flow_idx;
 	if (table->resource) {
 		mlx5_ipool_malloc(table->resource, &res_idx);
-		if (!res_idx)
+		if (!res_idx) {
+			rte_errno = ENOMEM;
 			goto error;
+		}
 		flow->res_idx = res_idx;
 	} else {
 		flow->res_idx = flow_idx;
@@ -4062,8 +4073,10 @@ flow_hw_async_flow_create_by_index(struct rte_eth_dev *dev,
 			return NULL;
 	}
 	flow = mlx5_ipool_malloc(table->flow, &flow_idx);
-	if (!flow)
+	if (!flow) {
+		rte_errno = ENOMEM;
 		goto error;
+	}
 	rule_acts = flow_hw_get_dr_action_buffer(priv, table, action_template_index, queue);
 	/*
 	 * Set the table here in order to know the destination table
@@ -4074,8 +4087,10 @@ flow_hw_async_flow_create_by_index(struct rte_eth_dev *dev,
 	flow->idx = flow_idx;
 	if (table->resource) {
 		mlx5_ipool_malloc(table->resource, &res_idx);
-		if (!res_idx)
+		if (!res_idx) {
+			rte_errno = ENOMEM;
 			goto error;
+		}
 		flow->res_idx = res_idx;
 	} else {
 		flow->res_idx = flow_idx;
@@ -4210,8 +4225,10 @@ flow_hw_async_flow_update(struct rte_eth_dev *dev,
 	nf->idx = of->idx;
 	if (table->resource) {
 		mlx5_ipool_malloc(table->resource, &res_idx);
-		if (!res_idx)
+		if (!res_idx) {
+			rte_errno = ENOMEM;
 			goto error;
+		}
 		nf->res_idx = res_idx;
 	} else {
 		nf->res_idx = of->res_idx;
Signature · 1 line
2.34.1
author
#6 lore raw

PATCH 2/3 net/mlx5/hws: add log for failing to create rule in HWS

In reply to Gavin Li

From: "Minggang Li (Gavin)" <[email protected]>

---
 drivers/net/mlx5/hws/mlx5dr_rule.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/net/mlx5/hws/mlx5dr_rule.c +6 -0
index 1edb7eac74..5d66d81ea5 100644
@@ -638,6 +638,7 @@ static int mlx5dr_rule_destroy_hws(struct mlx5dr_rule *rule,
 
 	/* Rule is not completed yet */
 	if (rule->status == MLX5DR_RULE_STATUS_CREATING) {
+		DR_LOG(NOTICE, "Cannot destroy, rule creation still in progress");
 		rte_errno = EBUSY;
 		return rte_errno;
 	}
@@ -806,12 +807,14 @@ static int mlx5dr_rule_enqueue_precheck(struct mlx5dr_rule *rule,
 	struct mlx5dr_context *ctx = rule->matcher->tbl->ctx;
 
 	if (unlikely(!attr->user_data)) {
+		DR_LOG(DEBUG, "User data must be provided for rule operations");
 		rte_errno = EINVAL;
 		return rte_errno;
 	}
 
 	/* Check if there is room in queue */
 	if (unlikely(mlx5dr_send_engine_full(&ctx->send_queue[attr->queue_id]))) {
+		DR_LOG(NOTICE, "No room in queue[%d]", attr->queue_id);
 		rte_errno = EBUSY;
 		return rte_errno;
 	}
@@ -823,6 +826,7 @@ static int mlx5dr_rule_enqueue_precheck_move(struct mlx5dr_rule *rule,
 					     struct mlx5dr_rule_attr *attr)
 {
 	if (unlikely(rule->status != MLX5DR_RULE_STATUS_CREATED)) {
+		DR_LOG(DEBUG, "Cannot move, rule status is invalid");
 		rte_errno = EINVAL;
 		return rte_errno;
 	}
@@ -835,6 +839,7 @@ static int mlx5dr_rule_enqueue_precheck_create(struct mlx5dr_rule *rule,
 {
 	if (unlikely(mlx5dr_matcher_is_in_resize(rule->matcher))) {
 		/* Matcher in resize - new rules are not allowed */
+		DR_LOG(NOTICE, "Resizing in progress, cannot create rule");
 		rte_errno = EAGAIN;
 		return rte_errno;
 	}
@@ -1068,6 +1073,7 @@ int mlx5dr_rule_hash_calculate(struct mlx5dr_matcher *matcher,
 	    mlx5dr_table_is_root(matcher->tbl) ||
 	    matcher->tbl->ctx->caps->access_index_mode == MLX5DR_MATCHER_INSERT_BY_HASH ||
 	    matcher->tbl->ctx->caps->flow_table_hash_type != MLX5_FLOW_TABLE_HASH_TYPE_CRC32) {
+		DR_LOG(DEBUG, "Matcher is not supported");
 		rte_errno = ENOTSUP;
 		return -rte_errno;
 	}
Signature · 1 line
2.34.1
author
#7 lore raw

PATCH 3/3 net/mlx5/hws: print CQE error syndrome and more information

In reply to Gavin Li

From: "Minggang Li (Gavin)" <[email protected]>

---
 drivers/net/mlx5/hws/mlx5dr_send.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/net/mlx5/hws/mlx5dr_send.c +8 -1
index 3022c50260..c931896a79 100644
@@ -598,8 +598,15 @@ static void mlx5dr_send_engine_poll_cq(struct mlx5dr_send_engine *queue,
 	    cqe_owner != sw_own)
 		return;
 
-	if (unlikely(cqe_opcode != MLX5_CQE_REQ))
+	if (unlikely(cqe_opcode != MLX5_CQE_REQ)) {
+		struct mlx5_err_cqe *err_cqe = (struct mlx5_err_cqe *)cqe;
+
+		DR_LOG(ERR, "CQE ERR:0x%x, Vender_ERR:0x%x, OP:0x%x, QPN:0x%x, WQE_CNT:0x%x",
+			err_cqe->syndrome, err_cqe->vendor_err_synd, cqe_opcode,
+			(rte_be_to_cpu_32(err_cqe->s_wqe_opcode_qpn) & 0xffffff),
+			rte_be_to_cpu_16(err_cqe->wqe_counter));
 		queue->err = true;
+	}
 
 	rte_io_rmb();
 
Signature · 1 line
2.34.1
#8 lore raw

v2 cover Error report improvement and fix

This patch set is to improve error handling in pmd and under layer.

Gavin Li (3):
net/mlx5: set rte errno if malloc failed
---
changelog:
v0->v1
- Fix typo in commit message
v1->v2
- Fix signoff warning
---
net/mlx5/hws: add log for failing to create rule in HWS
---
changelog:
v1->v2
- Fix signoff warning
---
net/mlx5/hws: print CQE error syndrome and more information
---
changelog:
v1->v2
- Fix typo in log message
- Fix signoff warning
---

 drivers/net/mlx5/hws/mlx5dr_rule.c |  6 ++++++
 drivers/net/mlx5/hws/mlx5dr_send.c |  9 ++++++++-
 drivers/net/mlx5/mlx5_flow_hw.c    | 31 +++++++++++++++++++++++-------
 3 files changed, 38 insertions(+), 8 deletions(-)
Signature · 1 line
2.34.1
#9 lore raw

PATCH v2 1/3 net/mlx5: set rte errno if malloc failed

rte_errno should be set if anything wrong happened in under layer so that user can figure out what's going on.

There were some cases that did not set it when ipool allocation failed. To fix the issue, set rte_errno to ENOMEM if mlx5_ipool_malloc failed to allocate ID.

Fixes c40c061a02 ("net/mlx5: add basic flow queue operation")
Fixes 48fbb0e93d ("net/mlx5: support flow meter mark indirect action with HWS")
---
 drivers/net/mlx5/mlx5_flow_hw.c | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)
drivers/net/mlx5/mlx5_flow_hw.c +24 -7
index a275154d4b..f34670b3ec 100644
@@ -1905,7 +1905,7 @@ flow_hw_meter_mark_alloc(struct rte_eth_dev *dev, uint32_t queue,
 	const struct rte_flow_action_meter_mark *meter_mark = action->conf;
 	struct mlx5_aso_mtr *aso_mtr;
 	struct mlx5_flow_meter_info *fm;
-	uint32_t mtr_id;
+	uint32_t mtr_id = 0;
 	uintptr_t handle = (uintptr_t)MLX5_INDIRECT_ACTION_TYPE_METER_MARK <<
 					MLX5_INDIRECT_ACTION_TYPE_OFFSET;
 
@@ -1917,8 +1917,15 @@ flow_hw_meter_mark_alloc(struct rte_eth_dev *dev, uint32_t queue,
 	if (meter_mark->profile == NULL)
 		return NULL;
 	aso_mtr = mlx5_ipool_malloc(pool->idx_pool, &mtr_id);
-	if (!aso_mtr)
+	if (!aso_mtr) {
+		rte_flow_error_set(error, ENOMEM,
+				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				   NULL,
+				   "failed to allocate aso meter entry");
+		if (mtr_id)
+			mlx5_ipool_free(pool->idx_pool, mtr_id);
 		return NULL;
+	}
 	/* Fill the flow meter parameters. */
 	aso_mtr->type = ASO_METER_INDIRECT;
 	fm = &aso_mtr->fm;
@@ -3926,8 +3933,10 @@ flow_hw_async_flow_create(struct rte_eth_dev *dev,
 			return NULL;
 	}
 	flow = mlx5_ipool_malloc(table->flow, &flow_idx);
-	if (!flow)
+	if (!flow) {
+		rte_errno = ENOMEM;
 		goto error;
+	}
 	rule_acts = flow_hw_get_dr_action_buffer(priv, table, action_template_index, queue);
 	/*
 	 * Set the table here in order to know the destination table
@@ -3938,8 +3947,10 @@ flow_hw_async_flow_create(struct rte_eth_dev *dev,
 	flow->idx = flow_idx;
 	if (table->resource) {
 		mlx5_ipool_malloc(table->resource, &res_idx);
-		if (!res_idx)
+		if (!res_idx) {
+			rte_errno = ENOMEM;
 			goto error;
+		}
 		flow->res_idx = res_idx;
 	} else {
 		flow->res_idx = flow_idx;
@@ -4070,8 +4081,10 @@ flow_hw_async_flow_create_by_index(struct rte_eth_dev *dev,
 			return NULL;
 	}
 	flow = mlx5_ipool_malloc(table->flow, &flow_idx);
-	if (!flow)
+	if (!flow) {
+		rte_errno = ENOMEM;
 		goto error;
+	}
 	rule_acts = flow_hw_get_dr_action_buffer(priv, table, action_template_index, queue);
 	/*
 	 * Set the table here in order to know the destination table
@@ -4082,8 +4095,10 @@ flow_hw_async_flow_create_by_index(struct rte_eth_dev *dev,
 	flow->idx = flow_idx;
 	if (table->resource) {
 		mlx5_ipool_malloc(table->resource, &res_idx);
-		if (!res_idx)
+		if (!res_idx) {
+			rte_errno = ENOMEM;
 			goto error;
+		}
 		flow->res_idx = res_idx;
 	} else {
 		flow->res_idx = flow_idx;
@@ -4218,8 +4233,10 @@ flow_hw_async_flow_update(struct rte_eth_dev *dev,
 	nf->idx = of->idx;
 	if (table->resource) {
 		mlx5_ipool_malloc(table->resource, &res_idx);
-		if (!res_idx)
+		if (!res_idx) {
+			rte_errno = ENOMEM;
 			goto error;
+		}
 		nf->res_idx = res_idx;
 	} else {
 		nf->res_idx = of->res_idx;
Signature · 1 line
2.34.1
#10 lore raw

PATCH v2 2/3 net/mlx5/hws: add log for failing to create rule in HWS

In reply to Minggang Li(Gavin)
---
 drivers/net/mlx5/hws/mlx5dr_rule.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/net/mlx5/hws/mlx5dr_rule.c +6 -0
index 1edb7eac74..5d66d81ea5 100644
@@ -638,6 +638,7 @@ static int mlx5dr_rule_destroy_hws(struct mlx5dr_rule *rule,
 
 	/* Rule is not completed yet */
 	if (rule->status == MLX5DR_RULE_STATUS_CREATING) {
+		DR_LOG(NOTICE, "Cannot destroy, rule creation still in progress");
 		rte_errno = EBUSY;
 		return rte_errno;
 	}
@@ -806,12 +807,14 @@ static int mlx5dr_rule_enqueue_precheck(struct mlx5dr_rule *rule,
 	struct mlx5dr_context *ctx = rule->matcher->tbl->ctx;
 
 	if (unlikely(!attr->user_data)) {
+		DR_LOG(DEBUG, "User data must be provided for rule operations");
 		rte_errno = EINVAL;
 		return rte_errno;
 	}
 
 	/* Check if there is room in queue */
 	if (unlikely(mlx5dr_send_engine_full(&ctx->send_queue[attr->queue_id]))) {
+		DR_LOG(NOTICE, "No room in queue[%d]", attr->queue_id);
 		rte_errno = EBUSY;
 		return rte_errno;
 	}
@@ -823,6 +826,7 @@ static int mlx5dr_rule_enqueue_precheck_move(struct mlx5dr_rule *rule,
 					     struct mlx5dr_rule_attr *attr)
 {
 	if (unlikely(rule->status != MLX5DR_RULE_STATUS_CREATED)) {
+		DR_LOG(DEBUG, "Cannot move, rule status is invalid");
 		rte_errno = EINVAL;
 		return rte_errno;
 	}
@@ -835,6 +839,7 @@ static int mlx5dr_rule_enqueue_precheck_create(struct mlx5dr_rule *rule,
 {
 	if (unlikely(mlx5dr_matcher_is_in_resize(rule->matcher))) {
 		/* Matcher in resize - new rules are not allowed */
+		DR_LOG(NOTICE, "Resizing in progress, cannot create rule");
 		rte_errno = EAGAIN;
 		return rte_errno;
 	}
@@ -1068,6 +1073,7 @@ int mlx5dr_rule_hash_calculate(struct mlx5dr_matcher *matcher,
 	    mlx5dr_table_is_root(matcher->tbl) ||
 	    matcher->tbl->ctx->caps->access_index_mode == MLX5DR_MATCHER_INSERT_BY_HASH ||
 	    matcher->tbl->ctx->caps->flow_table_hash_type != MLX5_FLOW_TABLE_HASH_TYPE_CRC32) {
+		DR_LOG(DEBUG, "Matcher is not supported");
 		rte_errno = ENOTSUP;
 		return -rte_errno;
 	}
Signature · 1 line
2.34.1
#11 lore raw

PATCH v2 3/3 net/mlx5/hws: print CQE error syndrome and more information

In reply to Minggang Li(Gavin)
---
 drivers/net/mlx5/hws/mlx5dr_send.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/net/mlx5/hws/mlx5dr_send.c +8 -1
index 3022c50260..e9abf3dddb 100644
@@ -598,8 +598,15 @@ static void mlx5dr_send_engine_poll_cq(struct mlx5dr_send_engine *queue,
 	    cqe_owner != sw_own)
 		return;
 
-	if (unlikely(cqe_opcode != MLX5_CQE_REQ))
+	if (unlikely(cqe_opcode != MLX5_CQE_REQ)) {
+		struct mlx5_err_cqe *err_cqe = (struct mlx5_err_cqe *)cqe;
+
+		DR_LOG(ERR, "CQE ERR:0x%x, Vendor_ERR:0x%x, OP:0x%x, QPN:0x%x, WQE_CNT:0x%x",
+			err_cqe->syndrome, err_cqe->vendor_err_synd, cqe_opcode,
+			(rte_be_to_cpu_32(err_cqe->s_wqe_opcode_qpn) & 0xffffff),
+			rte_be_to_cpu_16(err_cqe->wqe_counter));
 		queue->err = true;
+	}
 
 	rte_io_rmb();
 
Signature · 1 line
2.34.1
#12 lore raw

v3 cover Error report improvement and fix

This patch set is to improve error handling in pmd and under layer.

Gavin Li (3):
net/mlx5: set rte errno if malloc failed
---
changelog:
v0->v1
- Fix typo in commit message
v1->v2
- Fix checkpatch warning
---
net/mlx5/hws: add log for failing to create rule in HWS
---
changelog:
v1->v2
- Fix checkpatch warning
v2->v3
- Fix checkpatch warning
---
net/mlx5/hws: print CQE error syndrome and more information
---
changelog:
v1->v2
- Fix checkpatch warning
v2->v3
- Fix checkpatch warning
---

 drivers/net/mlx5/hws/mlx5dr_rule.c |  6 ++++++
 drivers/net/mlx5/hws/mlx5dr_send.c |  9 ++++++++-
 drivers/net/mlx5/mlx5_flow_hw.c    | 31 +++++++++++++++++++++++-------
 3 files changed, 38 insertions(+), 8 deletions(-)
Signature · 1 line
2.34.1
#13 lore raw

PATCH v3 1/3 net/mlx5: set rte errno if malloc failed

rte_errno should be set if anything wrong happened in under layer so that user can figure out what's going on.

There were some cases that did not set it when ipool allocation failed. To fix the issue, set rte_errno to ENOMEM if mlx5_ipool_malloc failed to allocate ID.

Fixes c40c061a022e ("net/mlx5: add basic flow queue operation")
Fixes 48fbb0e93d06 ("net/mlx5: support flow meter mark indirect action with HWS")
---
 drivers/net/mlx5/mlx5_flow_hw.c | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)
drivers/net/mlx5/mlx5_flow_hw.c +24 -7
index a275154d4b..f34670b3ec 100644
@@ -1905,7 +1905,7 @@ flow_hw_meter_mark_alloc(struct rte_eth_dev *dev, uint32_t queue,
 	const struct rte_flow_action_meter_mark *meter_mark = action->conf;
 	struct mlx5_aso_mtr *aso_mtr;
 	struct mlx5_flow_meter_info *fm;
-	uint32_t mtr_id;
+	uint32_t mtr_id = 0;
 	uintptr_t handle = (uintptr_t)MLX5_INDIRECT_ACTION_TYPE_METER_MARK <<
 					MLX5_INDIRECT_ACTION_TYPE_OFFSET;
 
@@ -1917,8 +1917,15 @@ flow_hw_meter_mark_alloc(struct rte_eth_dev *dev, uint32_t queue,
 	if (meter_mark->profile == NULL)
 		return NULL;
 	aso_mtr = mlx5_ipool_malloc(pool->idx_pool, &mtr_id);
-	if (!aso_mtr)
+	if (!aso_mtr) {
+		rte_flow_error_set(error, ENOMEM,
+				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				   NULL,
+				   "failed to allocate aso meter entry");
+		if (mtr_id)
+			mlx5_ipool_free(pool->idx_pool, mtr_id);
 		return NULL;
+	}
 	/* Fill the flow meter parameters. */
 	aso_mtr->type = ASO_METER_INDIRECT;
 	fm = &aso_mtr->fm;
@@ -3926,8 +3933,10 @@ flow_hw_async_flow_create(struct rte_eth_dev *dev,
 			return NULL;
 	}
 	flow = mlx5_ipool_malloc(table->flow, &flow_idx);
-	if (!flow)
+	if (!flow) {
+		rte_errno = ENOMEM;
 		goto error;
+	}
 	rule_acts = flow_hw_get_dr_action_buffer(priv, table, action_template_index, queue);
 	/*
 	 * Set the table here in order to know the destination table
@@ -3938,8 +3947,10 @@ flow_hw_async_flow_create(struct rte_eth_dev *dev,
 	flow->idx = flow_idx;
 	if (table->resource) {
 		mlx5_ipool_malloc(table->resource, &res_idx);
-		if (!res_idx)
+		if (!res_idx) {
+			rte_errno = ENOMEM;
 			goto error;
+		}
 		flow->res_idx = res_idx;
 	} else {
 		flow->res_idx = flow_idx;
@@ -4070,8 +4081,10 @@ flow_hw_async_flow_create_by_index(struct rte_eth_dev *dev,
 			return NULL;
 	}
 	flow = mlx5_ipool_malloc(table->flow, &flow_idx);
-	if (!flow)
+	if (!flow) {
+		rte_errno = ENOMEM;
 		goto error;
+	}
 	rule_acts = flow_hw_get_dr_action_buffer(priv, table, action_template_index, queue);
 	/*
 	 * Set the table here in order to know the destination table
@@ -4082,8 +4095,10 @@ flow_hw_async_flow_create_by_index(struct rte_eth_dev *dev,
 	flow->idx = flow_idx;
 	if (table->resource) {
 		mlx5_ipool_malloc(table->resource, &res_idx);
-		if (!res_idx)
+		if (!res_idx) {
+			rte_errno = ENOMEM;
 			goto error;
+		}
 		flow->res_idx = res_idx;
 	} else {
 		flow->res_idx = flow_idx;
@@ -4218,8 +4233,10 @@ flow_hw_async_flow_update(struct rte_eth_dev *dev,
 	nf->idx = of->idx;
 	if (table->resource) {
 		mlx5_ipool_malloc(table->resource, &res_idx);
-		if (!res_idx)
+		if (!res_idx) {
+			rte_errno = ENOMEM;
 			goto error;
+		}
 		nf->res_idx = res_idx;
 	} else {
 		nf->res_idx = of->res_idx;
Signature · 1 line
2.34.1
#14 lore raw

PATCH v3 2/3 net/mlx5/hws: add log for failing to create rule in HWS

In reply to Minggang Li(Gavin)

Add log messages about the reason why the flow was failed to be created.

---
 drivers/net/mlx5/hws/mlx5dr_rule.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/net/mlx5/hws/mlx5dr_rule.c +6 -0
index 1edb7eac74..5d66d81ea5 100644
@@ -638,6 +638,7 @@ static int mlx5dr_rule_destroy_hws(struct mlx5dr_rule *rule,
 
 	/* Rule is not completed yet */
 	if (rule->status == MLX5DR_RULE_STATUS_CREATING) {
+		DR_LOG(NOTICE, "Cannot destroy, rule creation still in progress");
 		rte_errno = EBUSY;
 		return rte_errno;
 	}
@@ -806,12 +807,14 @@ static int mlx5dr_rule_enqueue_precheck(struct mlx5dr_rule *rule,
 	struct mlx5dr_context *ctx = rule->matcher->tbl->ctx;
 
 	if (unlikely(!attr->user_data)) {
+		DR_LOG(DEBUG, "User data must be provided for rule operations");
 		rte_errno = EINVAL;
 		return rte_errno;
 	}
 
 	/* Check if there is room in queue */
 	if (unlikely(mlx5dr_send_engine_full(&ctx->send_queue[attr->queue_id]))) {
+		DR_LOG(NOTICE, "No room in queue[%d]", attr->queue_id);
 		rte_errno = EBUSY;
 		return rte_errno;
 	}
@@ -823,6 +826,7 @@ static int mlx5dr_rule_enqueue_precheck_move(struct mlx5dr_rule *rule,
 					     struct mlx5dr_rule_attr *attr)
 {
 	if (unlikely(rule->status != MLX5DR_RULE_STATUS_CREATED)) {
+		DR_LOG(DEBUG, "Cannot move, rule status is invalid");
 		rte_errno = EINVAL;
 		return rte_errno;
 	}
@@ -835,6 +839,7 @@ static int mlx5dr_rule_enqueue_precheck_create(struct mlx5dr_rule *rule,
 {
 	if (unlikely(mlx5dr_matcher_is_in_resize(rule->matcher))) {
 		/* Matcher in resize - new rules are not allowed */
+		DR_LOG(NOTICE, "Resizing in progress, cannot create rule");
 		rte_errno = EAGAIN;
 		return rte_errno;
 	}
@@ -1068,6 +1073,7 @@ int mlx5dr_rule_hash_calculate(struct mlx5dr_matcher *matcher,
 	    mlx5dr_table_is_root(matcher->tbl) ||
 	    matcher->tbl->ctx->caps->access_index_mode == MLX5DR_MATCHER_INSERT_BY_HASH ||
 	    matcher->tbl->ctx->caps->flow_table_hash_type != MLX5_FLOW_TABLE_HASH_TYPE_CRC32) {
+		DR_LOG(DEBUG, "Matcher is not supported");
 		rte_errno = ENOTSUP;
 		return -rte_errno;
 	}
Signature · 1 line
2.34.1
#15 lore raw

PATCH v3 3/3 net/mlx5/hws: print CQE error syndrome and more information

In reply to Minggang Li(Gavin)

Print CQE error syndrome and more information in case of queue error.

---
 drivers/net/mlx5/hws/mlx5dr_send.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/net/mlx5/hws/mlx5dr_send.c +8 -1
index 3022c50260..e9abf3dddb 100644
@@ -598,8 +598,15 @@ static void mlx5dr_send_engine_poll_cq(struct mlx5dr_send_engine *queue,
 	    cqe_owner != sw_own)
 		return;
 
-	if (unlikely(cqe_opcode != MLX5_CQE_REQ))
+	if (unlikely(cqe_opcode != MLX5_CQE_REQ)) {
+		struct mlx5_err_cqe *err_cqe = (struct mlx5_err_cqe *)cqe;
+
+		DR_LOG(ERR, "CQE ERR:0x%x, Vendor_ERR:0x%x, OP:0x%x, QPN:0x%x, WQE_CNT:0x%x",
+			err_cqe->syndrome, err_cqe->vendor_err_synd, cqe_opcode,
+			(rte_be_to_cpu_32(err_cqe->s_wqe_opcode_qpn) & 0xffffff),
+			rte_be_to_cpu_16(err_cqe->wqe_counter));
 		queue->err = true;
+	}
 
 	rte_io_rmb();
 
Signature · 1 line
2.34.1
#16 lore raw
In reply to Minggang Li(Gavin)

Hi,

From: Minggang(Gavin) Li <[email protected]>
Sent: Tuesday, September 24, 2024 1:52 PM
To: Matan Azrad; Slava Ovsiienko; Ori Kam; NBU-Contact-Thomas Monjalon (EXTERNAL)
Cc: [email protected]; Raslan Darawsheh
Subject: [PATCH V3 0/3] Error report improvement and fix

This patch set is to improve error handling in pmd and under layer.

Gavin Li (3):
net/mlx5: set rte errno if malloc failed
---
changelog:
v0->v1
- Fix typo in commit message
v1->v2
- Fix checkpatch warning
---
net/mlx5/hws: add log for failing to create rule in HWS
---
changelog:
v1->v2
- Fix checkpatch warning
v2->v3
- Fix checkpatch warning
---
net/mlx5/hws: print CQE error syndrome and more information
---
changelog:
v1->v2
- Fix checkpatch warning
v2->v3
- Fix checkpatch warning
---

 drivers/net/mlx5/hws/mlx5dr_rule.c |  6 ++++++
 drivers/net/mlx5/hws/mlx5dr_send.c |  9 ++++++++-
 drivers/net/mlx5/mlx5_flow_hw.c    | 31 +++++++++++++++++++++++-------
 3 files changed, 38 insertions(+), 8 deletions(-)

--
2.34.1

Series applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh