DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v1 0/4] net/mlx4: RSS fixes
@ 2017-10-19 16:11 Adrien Mazarguil
  2017-10-19 16:11 ` [dpdk-dev] [PATCH v1 1/4] net/mlx4: use dedicated list iterator Adrien Mazarguil
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Adrien Mazarguil @ 2017-10-19 16:11 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Nelio Laranjeiro, dev

This series enforces WQ creation order to make sure WQNs are assigned
sequentially to Rx queues and fixes a few minor issues related to
initialization code.

This addresses l3fwd-power startup issues.

Adrien Mazarguil (4):
  net/mlx4: use dedicated list iterator
  net/mlx4: fix useless flow rules synchronization
  net/mlx4: fix indirection table error rollback
  net/mlx4: relax Rx queue configuration order

 drivers/net/mlx4/mlx4.c      |  23 +--
 drivers/net/mlx4/mlx4_flow.c |   4 +-
 drivers/net/mlx4/mlx4_rxq.c  | 388 ++++++++++++++++++++++++++++----------
 drivers/net/mlx4/mlx4_rxtx.h |   5 +
 4 files changed, 307 insertions(+), 113 deletions(-)

-- 
2.1.4

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

* [dpdk-dev] [PATCH v1 1/4] net/mlx4: use dedicated list iterator
  2017-10-19 16:11 [dpdk-dev] [PATCH v1 0/4] net/mlx4: RSS fixes Adrien Mazarguil
@ 2017-10-19 16:11 ` Adrien Mazarguil
  2017-10-19 16:11 ` [dpdk-dev] [PATCH v1 2/4] net/mlx4: fix useless flow rules synchronization Adrien Mazarguil
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Adrien Mazarguil @ 2017-10-19 16:11 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Nelio Laranjeiro, dev

Dumb unconditional iteration on flow rules should be performed using the
dedicated macro.

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 drivers/net/mlx4/mlx4_flow.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/mlx4/mlx4_flow.c b/drivers/net/mlx4/mlx4_flow.c
index 5c4bf8e..5af6efb 100644
--- a/drivers/net/mlx4/mlx4_flow.c
+++ b/drivers/net/mlx4/mlx4_flow.c
@@ -1460,9 +1460,7 @@ mlx4_flow_sync(struct priv *priv, struct rte_flow_error *error)
 			return ret;
 	}
 	/* Toggle the remaining flow rules . */
-	for (flow = LIST_FIRST(&priv->flows);
-	     flow;
-	     flow = LIST_NEXT(flow, next)) {
+	LIST_FOREACH(flow, &priv->flows, next) {
 		ret = mlx4_flow_toggle(priv, flow, priv->started, error);
 		if (ret)
 			return ret;
-- 
2.1.4

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

* [dpdk-dev] [PATCH v1 2/4] net/mlx4: fix useless flow rules synchronization
  2017-10-19 16:11 [dpdk-dev] [PATCH v1 0/4] net/mlx4: RSS fixes Adrien Mazarguil
  2017-10-19 16:11 ` [dpdk-dev] [PATCH v1 1/4] net/mlx4: use dedicated list iterator Adrien Mazarguil
@ 2017-10-19 16:11 ` Adrien Mazarguil
  2017-10-19 16:11 ` [dpdk-dev] [PATCH v1 3/4] net/mlx4: fix indirection table error rollback Adrien Mazarguil
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Adrien Mazarguil @ 2017-10-19 16:11 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Nelio Laranjeiro, dev

According to the original commit, Rx queues cannot be created nor destroyed
while the device is started. Synchronizing flow rules during such events is
unnecessary as it occurs later when starting the device.

Fixes: 79770826499b ("net/mlx4: drop live queue reconfiguration support")

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 drivers/net/mlx4/mlx4_rxq.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/net/mlx4/mlx4_rxq.c b/drivers/net/mlx4/mlx4_rxq.c
index 6fa48bc..65cf123 100644
--- a/drivers/net/mlx4/mlx4_rxq.c
+++ b/drivers/net/mlx4/mlx4_rxq.c
@@ -407,7 +407,6 @@ mlx4_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc,
 	struct mlx4dv_cq dv_cq;
 	uint32_t mb_len = rte_pktmbuf_data_room_size(mp);
 	struct rte_mbuf *(*elts)[rte_align32pow2(desc)];
-	struct rte_flow_error error;
 	struct rxq *rxq;
 	struct mlx4_malloc_vec vec[] = {
 		{
@@ -609,19 +608,11 @@ mlx4_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc,
 	}
 	DEBUG("%p: adding Rx queue %p to list", (void *)dev, (void *)rxq);
 	dev->data->rx_queues[idx] = rxq;
-	/* Enable associated flows. */
-	ret = mlx4_flow_sync(priv, &error);
-	if (!ret) {
-		/* Update doorbell counter. */
-		rxq->rq_ci = desc >> rxq->sges_n;
-		rte_wmb();
-		*rxq->rq_db = rte_cpu_to_be_32(rxq->rq_ci);
-		return 0;
-	}
-	ERROR("cannot re-attach flow rules to queue %u"
-	      " (code %d, \"%s\"), flow error type %d, cause %p, message: %s",
-	      idx, -ret, strerror(-ret), error.type, error.cause,
-	      error.message ? error.message : "(unspecified)");
+	/* Update doorbell counter. */
+	rxq->rq_ci = desc >> rxq->sges_n;
+	rte_wmb();
+	*rxq->rq_db = rte_cpu_to_be_32(rxq->rq_ci);
+	return 0;
 error:
 	dev->data->rx_queues[idx] = NULL;
 	ret = rte_errno;
@@ -654,7 +645,6 @@ mlx4_rx_queue_release(void *dpdk_rxq)
 			priv->dev->data->rx_queues[i] = NULL;
 			break;
 		}
-	mlx4_flow_sync(priv, NULL);
 	mlx4_rxq_free_elts(rxq);
 	if (rxq->wq)
 		claim_zero(ibv_destroy_wq(rxq->wq));
-- 
2.1.4

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

* [dpdk-dev] [PATCH v1 3/4] net/mlx4: fix indirection table error rollback
  2017-10-19 16:11 [dpdk-dev] [PATCH v1 0/4] net/mlx4: RSS fixes Adrien Mazarguil
  2017-10-19 16:11 ` [dpdk-dev] [PATCH v1 1/4] net/mlx4: use dedicated list iterator Adrien Mazarguil
  2017-10-19 16:11 ` [dpdk-dev] [PATCH v1 2/4] net/mlx4: fix useless flow rules synchronization Adrien Mazarguil
@ 2017-10-19 16:11 ` Adrien Mazarguil
  2017-10-19 16:11 ` [dpdk-dev] [PATCH v1 4/4] net/mlx4: relax Rx queue configuration order Adrien Mazarguil
  2017-10-23 19:18 ` [dpdk-dev] [PATCH v1 0/4] net/mlx4: RSS fixes Ferruh Yigit
  4 siblings, 0 replies; 6+ messages in thread
From: Adrien Mazarguil @ 2017-10-19 16:11 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Nelio Laranjeiro, dev

In case of error occurring while setting up indirection table and related
RSS context resources, intermediate objects are not cleaned up.

Moreover although unlikely, an error other than EINVAL (e.g. ENOMEM) may be
returned.

A description of mlx4_rss_attach()'s return value is also missing.

Fixes: 078b8b452e6b ("net/mlx4: add RSS flow rule action support")

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 drivers/net/mlx4/mlx4_rxq.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/net/mlx4/mlx4_rxq.c b/drivers/net/mlx4/mlx4_rxq.c
index 65cf123..ee29556 100644
--- a/drivers/net/mlx4/mlx4_rxq.c
+++ b/drivers/net/mlx4/mlx4_rxq.c
@@ -185,6 +185,9 @@ void mlx4_rss_put(struct mlx4_rss *rss)
  *
  * @param rss
  *   RSS context to attach to.
+ *
+ * @return
+ *   0 on success, a negative errno value otherwise and rte_errno is set.
  */
 int mlx4_rss_attach(struct mlx4_rss *rss)
 {
@@ -202,6 +205,7 @@ int mlx4_rss_attach(struct mlx4_rss *rss)
 	int ret;
 
 	if (!rte_is_power_of_2(RTE_DIM(ind_tbl))) {
+		ret = EINVAL;
 		msg = "number of RSS queues must be a power of two";
 		goto error;
 	}
@@ -212,6 +216,7 @@ int mlx4_rss_attach(struct mlx4_rss *rss)
 		if (id < priv->dev->data->nb_rx_queues)
 			rxq = priv->dev->data->rx_queues[id];
 		if (!rxq) {
+			ret = EINVAL;
 			msg = "RSS target queue is not configured";
 			goto error;
 		}
@@ -225,6 +230,7 @@ int mlx4_rss_attach(struct mlx4_rss *rss)
 			.comp_mask = 0,
 		 });
 	if (!rss->ind) {
+		ret = errno ? errno : EINVAL;
 		msg = "RSS indirection table creation failure";
 		goto error;
 	}
@@ -245,6 +251,7 @@ int mlx4_rss_attach(struct mlx4_rss *rss)
 			},
 		 });
 	if (!rss->qp) {
+		ret = errno ? errno : EINVAL;
 		msg = "RSS hash QP creation failure";
 		goto error;
 	}
@@ -271,10 +278,18 @@ int mlx4_rss_attach(struct mlx4_rss *rss)
 	}
 	return 0;
 error:
+	if (rss->qp) {
+		claim_zero(ibv_destroy_qp(rss->qp));
+		rss->qp = NULL;
+	}
+	if (rss->ind) {
+		claim_zero(ibv_destroy_rwq_ind_table(rss->ind));
+		rss->ind = NULL;
+	}
 	ERROR("mlx4: %s", msg);
 	--rss->usecnt;
-	rte_errno = EINVAL;
-	return -rte_errno;
+	rte_errno = ret;
+	return -ret;
 }
 
 /**
-- 
2.1.4

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

* [dpdk-dev] [PATCH v1 4/4] net/mlx4: relax Rx queue configuration order
  2017-10-19 16:11 [dpdk-dev] [PATCH v1 0/4] net/mlx4: RSS fixes Adrien Mazarguil
                   ` (2 preceding siblings ...)
  2017-10-19 16:11 ` [dpdk-dev] [PATCH v1 3/4] net/mlx4: fix indirection table error rollback Adrien Mazarguil
@ 2017-10-19 16:11 ` Adrien Mazarguil
  2017-10-23 19:18 ` [dpdk-dev] [PATCH v1 0/4] net/mlx4: RSS fixes Ferruh Yigit
  4 siblings, 0 replies; 6+ messages in thread
From: Adrien Mazarguil @ 2017-10-19 16:11 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Nelio Laranjeiro, dev

Various hardware limitations apply to RSS indirection tables, one of them
being they must be an exact 1:1 mapping of the configured Rx queue indices.

While this restriction is enforced when creating RSS flow rules, it is not
the case when Rx queues themselves are created; underlying WQ numbers are
assigned in turn, not according to queue index.

Applications such as l3fwd-power that create Rx queues from highest to
lowest index (or any other non-sequential order) thus fail to get a working
RSS context.

This commit postpones WQ initialization to dev_start(), once all Rx queues
are configured in order to address this issue.

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 drivers/net/mlx4/mlx4.c      |  23 +--
 drivers/net/mlx4/mlx4_rxq.c  | 357 +++++++++++++++++++++++++++++---------
 drivers/net/mlx4/mlx4_rxtx.h |   5 +
 3 files changed, 288 insertions(+), 97 deletions(-)

diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index a297b9a..9be875b 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -50,7 +50,6 @@
 #pragma GCC diagnostic ignored "-Wpedantic"
 #endif
 #include <infiniband/verbs.h>
-#include <infiniband/mlx4dv.h>
 #ifdef PEDANTIC
 #pragma GCC diagnostic error "-Wpedantic"
 #endif
@@ -100,20 +99,8 @@ mlx4_dev_configure(struct rte_eth_dev *dev)
 {
 	struct priv *priv = dev->data->dev_private;
 	struct rte_flow_error error;
-	uint8_t log2_range = rte_log2_u32(dev->data->nb_rx_queues);
 	int ret;
 
-	/* Prepare range for RSS contexts before creating the first WQ. */
-	ret = mlx4dv_set_context_attr(priv->ctx,
-				      MLX4DV_SET_CTX_ATTR_LOG_WQS_RANGE_SZ,
-				      &log2_range);
-	if (ret) {
-		ERROR("cannot set up range size for RSS context to %u"
-		      " (for %u Rx queues), error: %s",
-		      1 << log2_range, dev->data->nb_rx_queues, strerror(ret));
-		rte_errno = ret;
-		return -ret;
-	}
 	/* Prepare internal flow rules. */
 	ret = mlx4_flow_sync(priv, &error);
 	if (ret) {
@@ -128,7 +115,8 @@ mlx4_dev_configure(struct rte_eth_dev *dev)
 /**
  * DPDK callback to start the device.
  *
- * Simulate device start by attaching all configured flows.
+ * Simulate device start by initializing common RSS resources and attaching
+ * all configured flows.
  *
  * @param dev
  *   Pointer to Ethernet device structure.
@@ -147,6 +135,12 @@ mlx4_dev_start(struct rte_eth_dev *dev)
 		return 0;
 	DEBUG("%p: attaching configured flows to all RX queues", (void *)dev);
 	priv->started = 1;
+	ret = mlx4_rss_init(priv);
+	if (ret) {
+		ERROR("%p: cannot initialize RSS resources: %s",
+		      (void *)dev, strerror(-ret));
+		goto err;
+	}
 	ret = mlx4_intr_install(priv);
 	if (ret) {
 		ERROR("%p: interrupt handler installation failed",
@@ -194,6 +188,7 @@ mlx4_dev_stop(struct rte_eth_dev *dev)
 	rte_wmb();
 	mlx4_flow_sync(priv, NULL);
 	mlx4_intr_uninstall(priv);
+	mlx4_rss_deinit(priv);
 }
 
 /**
diff --git a/drivers/net/mlx4/mlx4_rxq.c b/drivers/net/mlx4/mlx4_rxq.c
index ee29556..fb28290 100644
--- a/drivers/net/mlx4/mlx4_rxq.c
+++ b/drivers/net/mlx4/mlx4_rxq.c
@@ -46,6 +46,7 @@
 #ifdef PEDANTIC
 #pragma GCC diagnostic ignored "-Wpedantic"
 #endif
+#include <infiniband/mlx4dv.h>
 #include <infiniband/verbs.h>
 #ifdef PEDANTIC
 #pragma GCC diagnostic error "-Wpedantic"
@@ -201,7 +202,7 @@ int mlx4_rss_attach(struct mlx4_rss *rss)
 	struct ibv_wq *ind_tbl[rss->queues];
 	struct priv *priv = rss->priv;
 	const char *msg;
-	unsigned int i;
+	unsigned int i = 0;
 	int ret;
 
 	if (!rte_is_power_of_2(RTE_DIM(ind_tbl))) {
@@ -220,6 +221,12 @@ int mlx4_rss_attach(struct mlx4_rss *rss)
 			msg = "RSS target queue is not configured";
 			goto error;
 		}
+		ret = mlx4_rxq_attach(rxq);
+		if (ret) {
+			ret = -ret;
+			msg = "unable to attach RSS target queue";
+			goto error;
+		}
 		ind_tbl[i] = rxq->wq;
 	}
 	rss->ind = ibv_create_rwq_ind_table
@@ -286,6 +293,8 @@ int mlx4_rss_attach(struct mlx4_rss *rss)
 		claim_zero(ibv_destroy_rwq_ind_table(rss->ind));
 		rss->ind = NULL;
 	}
+	while (i--)
+		mlx4_rxq_detach(priv->dev->data->rx_queues[rss->queue_id[i]]);
 	ERROR("mlx4: %s", msg);
 	--rss->usecnt;
 	rte_errno = ret;
@@ -305,6 +314,9 @@ int mlx4_rss_attach(struct mlx4_rss *rss)
  */
 void mlx4_rss_detach(struct mlx4_rss *rss)
 {
+	struct priv *priv = rss->priv;
+	unsigned int i;
+
 	assert(rss->refcnt);
 	assert(rss->qp);
 	assert(rss->ind);
@@ -314,10 +326,164 @@ void mlx4_rss_detach(struct mlx4_rss *rss)
 	rss->qp = NULL;
 	claim_zero(ibv_destroy_rwq_ind_table(rss->ind));
 	rss->ind = NULL;
+	for (i = 0; i != rss->queues; ++i)
+		mlx4_rxq_detach(priv->dev->data->rx_queues[rss->queue_id[i]]);
 }
 
 /**
- * Allocate Rx queue elements.
+ * Initialize common RSS context resources.
+ *
+ * Because ConnectX-3 hardware limitations require a fixed order in the
+ * indirection table, WQs must be allocated sequentially to be part of a
+ * common RSS context.
+ *
+ * Since a newly created WQ cannot be moved to a different context, this
+ * function allocates them all at once, one for each configured Rx queue,
+ * as well as all related resources (CQs and mbufs).
+ *
+ * This must therefore be done before creating any Rx flow rules relying on
+ * indirection tables.
+ *
+ * @param priv
+ *   Pointer to private structure.
+ *
+ * @return
+ *   0 on success, a negative errno value otherwise and rte_errno is set.
+ */
+int
+mlx4_rss_init(struct priv *priv)
+{
+	struct rte_eth_dev *dev = priv->dev;
+	uint8_t log2_range = rte_log2_u32(dev->data->nb_rx_queues);
+	uint32_t wq_num_prev = 0;
+	const char *msg;
+	unsigned int i;
+	int ret;
+
+	/* Prepare range for RSS contexts before creating the first WQ. */
+	ret = mlx4dv_set_context_attr(priv->ctx,
+				      MLX4DV_SET_CTX_ATTR_LOG_WQS_RANGE_SZ,
+				      &log2_range);
+	if (ret) {
+		ERROR("cannot set up range size for RSS context to %u"
+		      " (for %u Rx queues), error: %s",
+		      1 << log2_range, dev->data->nb_rx_queues, strerror(ret));
+		rte_errno = ret;
+		return -ret;
+	}
+	for (i = 0; i != priv->dev->data->nb_rx_queues; ++i) {
+		struct rxq *rxq = priv->dev->data->rx_queues[i];
+		struct ibv_cq *cq;
+		struct ibv_wq *wq;
+		uint32_t wq_num;
+
+		/* Attach the configured Rx queues. */
+		if (rxq) {
+			assert(!rxq->usecnt);
+			ret = mlx4_rxq_attach(rxq);
+			if (!ret) {
+				wq_num = rxq->wq->wq_num;
+				goto wq_num_check;
+			}
+			ret = -ret;
+			msg = "unable to create Rx queue resources";
+			goto error;
+		}
+		/*
+		 * WQs are temporarily allocated for unconfigured Rx queues
+		 * to maintain proper index alignment in indirection table
+		 * by skipping unused WQ numbers.
+		 *
+		 * The reason this works at all even though these WQs are
+		 * immediately destroyed is that WQNs are allocated
+		 * sequentially and are guaranteed to never be reused in the
+		 * same context by the underlying implementation.
+		 */
+		cq = ibv_create_cq(priv->ctx, 1, NULL, NULL, 0);
+		if (!cq) {
+			ret = ENOMEM;
+			msg = "placeholder CQ creation failure";
+			goto error;
+		}
+		wq = ibv_create_wq
+			(priv->ctx,
+			 &(struct ibv_wq_init_attr){
+				.wq_type = IBV_WQT_RQ,
+				.max_wr = 1,
+				.max_sge = 1,
+				.pd = priv->pd,
+				.cq = cq,
+			 });
+		if (wq) {
+			wq_num = wq->wq_num;
+			claim_zero(ibv_destroy_wq(wq));
+		}
+		claim_zero(ibv_destroy_cq(cq));
+		if (!wq) {
+			ret = ENOMEM;
+			msg = "placeholder WQ creation failure";
+			goto error;
+		}
+wq_num_check:
+		/*
+		 * While guaranteed by the implementation, make sure WQ
+		 * numbers are really sequential (as the saying goes,
+		 * trust, but verify).
+		 */
+		if (i && wq_num - wq_num_prev != 1) {
+			if (rxq)
+				mlx4_rxq_detach(rxq);
+			ret = ERANGE;
+			msg = "WQ numbers are not sequential";
+			goto error;
+		}
+		wq_num_prev = wq_num;
+	}
+	return 0;
+error:
+	ERROR("cannot initialize common RSS resources (queue %u): %s: %s",
+	      i, msg, strerror(ret));
+	while (i--) {
+		struct rxq *rxq = priv->dev->data->rx_queues[i];
+
+		if (rxq)
+			mlx4_rxq_detach(rxq);
+	}
+	rte_errno = ret;
+	return -ret;
+}
+
+/**
+ * Release common RSS context resources.
+ *
+ * As the reverse of mlx4_rss_init(), this must be done after removing all
+ * flow rules relying on indirection tables.
+ *
+ * @param priv
+ *   Pointer to private structure.
+ */
+void
+mlx4_rss_deinit(struct priv *priv)
+{
+	unsigned int i;
+
+	for (i = 0; i != priv->dev->data->nb_rx_queues; ++i) {
+		struct rxq *rxq = priv->dev->data->rx_queues[i];
+
+		if (rxq) {
+			assert(rxq->usecnt == 1);
+			mlx4_rxq_detach(rxq);
+		}
+	}
+}
+
+/**
+ * Attach a user to a Rx queue.
+ *
+ * Used when the resources of an Rx queue must be instantiated for it to
+ * become in a usable state.
+ *
+ * This function increments the usage count of the Rx queue.
  *
  * @param rxq
  *   Pointer to Rx queue structure.
@@ -325,17 +491,76 @@ void mlx4_rss_detach(struct mlx4_rss *rss)
  * @return
  *   0 on success, negative errno value otherwise and rte_errno is set.
  */
-static int
-mlx4_rxq_alloc_elts(struct rxq *rxq)
+int
+mlx4_rxq_attach(struct rxq *rxq)
 {
+	if (rxq->usecnt++) {
+		assert(rxq->cq);
+		assert(rxq->wq);
+		assert(rxq->wqes);
+		assert(rxq->rq_db);
+		return 0;
+	}
+
+	struct priv *priv = rxq->priv;
 	const uint32_t elts_n = 1 << rxq->elts_n;
 	const uint32_t sges_n = 1 << rxq->sges_n;
 	struct rte_mbuf *(*elts)[elts_n] = rxq->elts;
+	struct mlx4dv_obj mlxdv;
+	struct mlx4dv_rwq dv_rwq;
+	struct mlx4dv_cq dv_cq;
+	const char *msg;
+	struct ibv_cq *cq = NULL;
+	struct ibv_wq *wq = NULL;
+	volatile struct mlx4_wqe_data_seg (*wqes)[];
 	unsigned int i;
+	int ret;
 
 	assert(rte_is_power_of_2(elts_n));
+	cq = ibv_create_cq(priv->ctx, elts_n / sges_n, NULL, rxq->channel, 0);
+	if (!cq) {
+		ret = ENOMEM;
+		msg = "CQ creation failure";
+		goto error;
+	}
+	wq = ibv_create_wq
+		(priv->ctx,
+		 &(struct ibv_wq_init_attr){
+			.wq_type = IBV_WQT_RQ,
+			.max_wr = elts_n / sges_n,
+			.max_sge = sges_n,
+			.pd = priv->pd,
+			.cq = cq,
+		 });
+	if (!wq) {
+		ret = errno ? errno : EINVAL;
+		msg = "WQ creation failure";
+		goto error;
+	}
+	ret = ibv_modify_wq
+		(wq,
+		 &(struct ibv_wq_attr){
+			.attr_mask = IBV_WQ_ATTR_STATE,
+			.wq_state = IBV_WQS_RDY,
+		 });
+	if (ret) {
+		msg = "WQ state change to IBV_WQS_RDY failed";
+		goto error;
+	}
+	/* Retrieve device queue information. */
+	mlxdv.cq.in = cq;
+	mlxdv.cq.out = &dv_cq;
+	mlxdv.rwq.in = wq;
+	mlxdv.rwq.out = &dv_rwq;
+	ret = mlx4dv_init_obj(&mlxdv, MLX4DV_OBJ_RWQ | MLX4DV_OBJ_CQ);
+	if (ret) {
+		msg = "failed to obtain device information from WQ/CQ objects";
+		goto error;
+	}
+	wqes = (volatile struct mlx4_wqe_data_seg (*)[])
+		((uintptr_t)dv_rwq.buf.buf + dv_rwq.rq.offset);
 	for (i = 0; i != RTE_DIM(*elts); ++i) {
-		volatile struct mlx4_wqe_data_seg *scat = &(*rxq->wqes)[i];
+		volatile struct mlx4_wqe_data_seg *scat = &(*wqes)[i];
 		struct rte_mbuf *buf = rte_pktmbuf_alloc(rxq->mp);
 
 		if (buf == NULL) {
@@ -343,8 +568,9 @@ mlx4_rxq_alloc_elts(struct rxq *rxq)
 				rte_pktmbuf_free_seg((*elts)[i]);
 				(*elts)[i] = NULL;
 			}
-			rte_errno = ENOMEM;
-			return -rte_errno;
+			ret = ENOMEM;
+			msg = "cannot allocate mbuf";
+			goto error;
 		}
 		/* Headroom is reserved by rte_pktmbuf_alloc(). */
 		assert(buf->data_off == RTE_PKTMBUF_HEADROOM);
@@ -368,21 +594,55 @@ mlx4_rxq_alloc_elts(struct rxq *rxq)
 	}
 	DEBUG("%p: allocated and configured %u segments (max %u packets)",
 	      (void *)rxq, elts_n, elts_n / sges_n);
+	rxq->cq = cq;
+	rxq->wq = wq;
+	rxq->wqes = wqes;
+	rxq->rq_db = dv_rwq.rdb;
+	rxq->mcq.buf = dv_cq.buf.buf;
+	rxq->mcq.cqe_cnt = dv_cq.cqe_cnt;
+	rxq->mcq.set_ci_db = dv_cq.set_ci_db;
+	rxq->mcq.cqe_64 = (dv_cq.cqe_size & 64) ? 1 : 0;
+	/* Update doorbell counter. */
+	rxq->rq_ci = elts_n / sges_n;
+	rte_wmb();
+	*rxq->rq_db = rte_cpu_to_be_32(rxq->rq_ci);
 	return 0;
+error:
+	if (wq)
+		claim_zero(ibv_destroy_wq(wq));
+	if (cq)
+		claim_zero(ibv_destroy_cq(cq));
+	rte_errno = ret;
+	ERROR("error while attaching Rx queue %p: %s: %s",
+	      (void *)rxq, msg, strerror(ret));
+	return -ret;
 }
 
 /**
- * Free Rx queue elements.
+ * Detach a user from a Rx queue.
+ *
+ * This function decrements the usage count of the Rx queue and destroys
+ * usage resources after reaching 0.
  *
  * @param rxq
  *   Pointer to Rx queue structure.
  */
-static void
-mlx4_rxq_free_elts(struct rxq *rxq)
+void
+mlx4_rxq_detach(struct rxq *rxq)
 {
 	unsigned int i;
 	struct rte_mbuf *(*elts)[1 << rxq->elts_n] = rxq->elts;
 
+	if (--rxq->usecnt)
+		return;
+	rxq->rq_ci = 0;
+	memset(&rxq->mcq, 0, sizeof(rxq->mcq));
+	rxq->rq_db = NULL;
+	rxq->wqes = NULL;
+	claim_zero(ibv_destroy_wq(rxq->wq));
+	rxq->wq = NULL;
+	claim_zero(ibv_destroy_cq(rxq->cq));
+	rxq->cq = NULL;
 	DEBUG("%p: freeing Rx queue elements", (void *)rxq);
 	for (i = 0; (i != RTE_DIM(*elts)); ++i) {
 		if (!(*elts)[i])
@@ -417,9 +677,6 @@ mlx4_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc,
 		    struct rte_mempool *mp)
 {
 	struct priv *priv = dev->data->dev_private;
-	struct mlx4dv_obj mlxdv;
-	struct mlx4dv_rwq dv_rwq;
-	struct mlx4dv_cq dv_cq;
 	uint32_t mb_len = rte_pktmbuf_data_room_size(mp);
 	struct rte_mbuf *(*elts)[rte_align32pow2(desc)];
 	struct rxq *rxq;
@@ -560,73 +817,8 @@ mlx4_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc,
 			goto error;
 		}
 	}
-	rxq->cq = ibv_create_cq(priv->ctx, desc >> rxq->sges_n, NULL,
-				rxq->channel, 0);
-	if (!rxq->cq) {
-		rte_errno = ENOMEM;
-		ERROR("%p: CQ creation failure: %s",
-		      (void *)dev, strerror(rte_errno));
-		goto error;
-	}
-	rxq->wq = ibv_create_wq
-		(priv->ctx,
-		 &(struct ibv_wq_init_attr){
-			.wq_type = IBV_WQT_RQ,
-			.max_wr = desc >> rxq->sges_n,
-			.max_sge = 1 << rxq->sges_n,
-			.pd = priv->pd,
-			.cq = rxq->cq,
-		 });
-	if (!rxq->wq) {
-		rte_errno = errno ? errno : EINVAL;
-		ERROR("%p: WQ creation failure: %s",
-		      (void *)dev, strerror(rte_errno));
-		goto error;
-	}
-	ret = ibv_modify_wq
-		(rxq->wq,
-		 &(struct ibv_wq_attr){
-			.attr_mask = IBV_WQ_ATTR_STATE,
-			.wq_state = IBV_WQS_RDY,
-		 });
-	if (ret) {
-		rte_errno = ret;
-		ERROR("%p: WQ state to IBV_WPS_RDY failed: %s",
-		      (void *)dev, strerror(rte_errno));
-		goto error;
-	}
-	/* Retrieve device queue information. */
-	mlxdv.cq.in = rxq->cq;
-	mlxdv.cq.out = &dv_cq;
-	mlxdv.rwq.in = rxq->wq;
-	mlxdv.rwq.out = &dv_rwq;
-	ret = mlx4dv_init_obj(&mlxdv, MLX4DV_OBJ_RWQ | MLX4DV_OBJ_CQ);
-	if (ret) {
-		rte_errno = EINVAL;
-		ERROR("%p: failed to obtain device information", (void *)dev);
-		goto error;
-	}
-	rxq->wqes =
-		(volatile struct mlx4_wqe_data_seg (*)[])
-		((uintptr_t)dv_rwq.buf.buf + dv_rwq.rq.offset);
-	rxq->rq_db = dv_rwq.rdb;
-	rxq->rq_ci = 0;
-	rxq->mcq.buf = dv_cq.buf.buf;
-	rxq->mcq.cqe_cnt = dv_cq.cqe_cnt;
-	rxq->mcq.set_ci_db = dv_cq.set_ci_db;
-	rxq->mcq.cqe_64 = (dv_cq.cqe_size & 64) ? 1 : 0;
-	ret = mlx4_rxq_alloc_elts(rxq);
-	if (ret) {
-		ERROR("%p: RXQ allocation failed: %s",
-		      (void *)dev, strerror(rte_errno));
-		goto error;
-	}
 	DEBUG("%p: adding Rx queue %p to list", (void *)dev, (void *)rxq);
 	dev->data->rx_queues[idx] = rxq;
-	/* Update doorbell counter. */
-	rxq->rq_ci = desc >> rxq->sges_n;
-	rte_wmb();
-	*rxq->rq_db = rte_cpu_to_be_32(rxq->rq_ci);
 	return 0;
 error:
 	dev->data->rx_queues[idx] = NULL;
@@ -660,11 +852,10 @@ mlx4_rx_queue_release(void *dpdk_rxq)
 			priv->dev->data->rx_queues[i] = NULL;
 			break;
 		}
-	mlx4_rxq_free_elts(rxq);
-	if (rxq->wq)
-		claim_zero(ibv_destroy_wq(rxq->wq));
-	if (rxq->cq)
-		claim_zero(ibv_destroy_cq(rxq->cq));
+	assert(!rxq->cq);
+	assert(!rxq->wq);
+	assert(!rxq->wqes);
+	assert(!rxq->rq_db);
 	if (rxq->channel)
 		claim_zero(ibv_destroy_comp_channel(rxq->channel));
 	if (rxq->mr)
diff --git a/drivers/net/mlx4/mlx4_rxtx.h b/drivers/net/mlx4/mlx4_rxtx.h
index e10bbca..7d67748 100644
--- a/drivers/net/mlx4/mlx4_rxtx.h
+++ b/drivers/net/mlx4/mlx4_rxtx.h
@@ -83,6 +83,7 @@ struct rxq {
 	struct mlx4_cq mcq;  /**< Info for directly manipulating the CQ. */
 	struct mlx4_rxq_stats stats; /**< Rx queue counters. */
 	unsigned int socket; /**< CPU socket ID for allocations. */
+	uint32_t usecnt; /**< Number of users relying on queue resources. */
 	uint8_t data[]; /**< Remaining queue resources. */
 };
 
@@ -146,12 +147,16 @@ struct txq {
 /* mlx4_rxq.c */
 
 uint8_t mlx4_rss_hash_key_default[MLX4_RSS_HASH_KEY_SIZE];
+int mlx4_rss_init(struct priv *priv);
+void mlx4_rss_deinit(struct priv *priv);
 struct mlx4_rss *mlx4_rss_get(struct priv *priv, uint64_t fields,
 			      uint8_t key[MLX4_RSS_HASH_KEY_SIZE],
 			      uint16_t queues, const uint16_t queue_id[]);
 void mlx4_rss_put(struct mlx4_rss *rss);
 int mlx4_rss_attach(struct mlx4_rss *rss);
 void mlx4_rss_detach(struct mlx4_rss *rss);
+int mlx4_rxq_attach(struct rxq *rxq);
+void mlx4_rxq_detach(struct rxq *rxq);
 int mlx4_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx,
 			uint16_t desc, unsigned int socket,
 			const struct rte_eth_rxconf *conf,
-- 
2.1.4

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

* Re: [dpdk-dev] [PATCH v1 0/4] net/mlx4: RSS fixes
  2017-10-19 16:11 [dpdk-dev] [PATCH v1 0/4] net/mlx4: RSS fixes Adrien Mazarguil
                   ` (3 preceding siblings ...)
  2017-10-19 16:11 ` [dpdk-dev] [PATCH v1 4/4] net/mlx4: relax Rx queue configuration order Adrien Mazarguil
@ 2017-10-23 19:18 ` Ferruh Yigit
  4 siblings, 0 replies; 6+ messages in thread
From: Ferruh Yigit @ 2017-10-23 19:18 UTC (permalink / raw)
  To: Adrien Mazarguil; +Cc: Nelio Laranjeiro, dev

On 10/19/2017 9:11 AM, Adrien Mazarguil wrote:
> This series enforces WQ creation order to make sure WQNs are assigned
> sequentially to Rx queues and fixes a few minor issues related to
> initialization code.
> 
> This addresses l3fwd-power startup issues.
> 
> Adrien Mazarguil (4):
>   net/mlx4: use dedicated list iterator
>   net/mlx4: fix useless flow rules synchronization
>   net/mlx4: fix indirection table error rollback
>   net/mlx4: relax Rx queue configuration order

Series applied to dpdk-next-net/master, thanks.

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

end of thread, other threads:[~2017-10-23 19:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-19 16:11 [dpdk-dev] [PATCH v1 0/4] net/mlx4: RSS fixes Adrien Mazarguil
2017-10-19 16:11 ` [dpdk-dev] [PATCH v1 1/4] net/mlx4: use dedicated list iterator Adrien Mazarguil
2017-10-19 16:11 ` [dpdk-dev] [PATCH v1 2/4] net/mlx4: fix useless flow rules synchronization Adrien Mazarguil
2017-10-19 16:11 ` [dpdk-dev] [PATCH v1 3/4] net/mlx4: fix indirection table error rollback Adrien Mazarguil
2017-10-19 16:11 ` [dpdk-dev] [PATCH v1 4/4] net/mlx4: relax Rx queue configuration order Adrien Mazarguil
2017-10-23 19:18 ` [dpdk-dev] [PATCH v1 0/4] net/mlx4: RSS fixes Ferruh Yigit

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).