[dpdk-dev] [PATCH] net/mlx4: fix drop action setting before start

[Matan Azrad <matan@mellanox.com>]

The corrupted code causes segmentation fault when user creates
flow with drop action before device starting.

For example, failsafe PMD recreates all the flows before calling
dev_start in plug-in sequence and mlx4 allocated its flow drop
queue in dev_start.
Hence, when failsafe created flow with drop action after plug-in
event, mlx4 tried to dereference flow drop queue which was uninitialized.

The fix changed the device private structure to hold the flow drop
queue by value instead of by reference.
Hence, the flow drop queue dynamic allocation and free were removed, and
all the accesses to its internal fields were changed.

The segmentation fault should not occur anymore because the memory
of flow drop queue is always allocated in configuration time.

Fixes: 642fe56a1ba5 ("net/mlx4: use a single drop queue for all drop flows")
Cc: stable@dpdk.org

Signed-off-by: Matan Azrad <matan@mellanox.com>
---
 drivers/net/mlx4/mlx4.h      |  7 ++++++-
 drivers/net/mlx4/mlx4_flow.c | 32 +++++++-------------------------
 2 files changed, 13 insertions(+), 26 deletions(-)

diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h
index a2e0ae7..ecaab35 100644
--- a/drivers/net/mlx4/mlx4.h
+++ b/drivers/net/mlx4/mlx4.h
@@ -309,6 +309,11 @@ struct txq {
 
 struct rte_flow;
 
+struct rte_flow_drop {
+	struct ibv_qp *qp; /**< Verbs queue pair. */
+	struct ibv_cq *cq; /**< Verbs completion queue. */
+};
+
 struct priv {
 	struct rte_eth_dev *dev; /* Ethernet device. */
 	struct ibv_context *ctx; /* Verbs context. */
@@ -352,7 +357,7 @@ struct priv {
 	struct txq *(*txqs)[]; /* TX queues. */
 	struct rte_intr_handle intr_handle_dev; /* Device interrupt handler. */
 	struct rte_intr_handle intr_handle; /* Interrupt handler. */
-	struct rte_flow_drop *flow_drop_queue; /* Flow drop queue. */
+	struct rte_flow_drop flow_drop_queue; /* Flow drop queue. */
 	LIST_HEAD(mlx4_flows, rte_flow) flows;
 	struct rte_intr_conf intr_conf; /* Active interrupt configuration. */
 	LIST_HEAD(mlx4_parents, rxq) parents;
diff --git a/drivers/net/mlx4/mlx4_flow.c b/drivers/net/mlx4/mlx4_flow.c
index b998bb9..a398f46 100644
--- a/drivers/net/mlx4/mlx4_flow.c
+++ b/drivers/net/mlx4/mlx4_flow.c
@@ -103,11 +103,6 @@ struct mlx4_flow_items {
 	const enum rte_flow_item_type *const items;
 };
 
-struct rte_flow_drop {
-	struct ibv_qp *qp; /**< Verbs queue pair. */
-	struct ibv_cq *cq; /**< Verbs completion queue. */
-};
-
 /** Valid action for this PMD. */
 static const enum rte_flow_action_type valid_actions[] = {
 	RTE_FLOW_ACTION_TYPE_DROP,
@@ -795,13 +790,9 @@ struct rte_flow_drop {
 static void
 mlx4_flow_destroy_drop_queue(struct priv *priv)
 {
-	if (priv->flow_drop_queue) {
-		struct rte_flow_drop *fdq = priv->flow_drop_queue;
-
-		priv->flow_drop_queue = NULL;
-		claim_zero(ibv_destroy_qp(fdq->qp));
-		claim_zero(ibv_destroy_cq(fdq->cq));
-		rte_free(fdq);
+	if (priv->flow_drop_queue.cq) {
+		claim_zero(ibv_destroy_qp(priv->flow_drop_queue.qp));
+		claim_zero(ibv_destroy_cq(priv->flow_drop_queue.cq));
 	}
 }
 
@@ -819,20 +810,14 @@ struct rte_flow_drop {
 {
 	struct ibv_qp *qp;
 	struct ibv_cq *cq;
-	struct rte_flow_drop *fdq;
 
-	fdq = rte_calloc(__func__, 1, sizeof(*fdq), 0);
-	if (!fdq) {
-		ERROR("Cannot allocate memory for drop struct");
-		goto err;
-	}
 	cq = ibv_exp_create_cq(priv->ctx, 1, NULL, NULL, 0,
 			      &(struct ibv_exp_cq_init_attr){
 					.comp_mask = 0,
 			      });
 	if (!cq) {
 		ERROR("Cannot create drop CQ");
-		goto err_create_cq;
+		goto err;
 	}
 	qp = ibv_exp_create_qp(priv->ctx,
 			      &(struct ibv_exp_qp_init_attr){
@@ -853,16 +838,13 @@ struct rte_flow_drop {
 		ERROR("Cannot create drop QP");
 		goto err_create_qp;
 	}
-	*fdq = (struct rte_flow_drop){
+	priv->flow_drop_queue = (struct rte_flow_drop){
 		.qp = qp,
 		.cq = cq,
 	};
-	priv->flow_drop_queue = fdq;
 	return 0;
 err_create_qp:
 	claim_zero(ibv_destroy_cq(cq));
-err_create_cq:
-	rte_free(fdq);
 err:
 	return -1;
 }
@@ -977,7 +959,7 @@ struct rte_flow_drop {
 		return NULL;
 	}
 	if (action->drop) {
-		qp = priv->flow_drop_queue->qp;
+		qp = priv->flow_drop_queue.qp;
 	} else {
 		int ret;
 		unsigned int i;
@@ -1307,7 +1289,7 @@ struct rte_flow *
 	for (flow = LIST_FIRST(&priv->flows);
 	     flow;
 	     flow = LIST_NEXT(flow, next)) {
-		qp = flow->qp ? flow->qp : priv->flow_drop_queue->qp;
+		qp = flow->qp ? flow->qp : priv->flow_drop_queue.qp;
 		flow->ibv_flow = ibv_create_flow(qp, flow->ibv_attr);
 		if (!flow->ibv_flow) {
 			DEBUG("Flow %p cannot be applied", (void *)flow);
-- 
1.8.3.1