[dpdk-dev] [PATCH v2] net/mlx5: fix Direct Verbs flow matcher caching

Ori Kam orika at mellanox.com
Thu Oct 4 07:53:40 CEST 2018


The Direct Verbs are using matcher object to filter flows, This object
can be reused for all flows that are using the same flow items and
masks.

This was implemented with an issue, that the list pointer pointed
to incorrect list type, this resulted in compilation error when using
GCC greater then 4.8.5

This commit fixes this issue by setting that the matcher object
will include the LIST required members directly and not as a subobject.

Fixes: 8d21c3d7b237 ("net/mlx5: add Direct Verbs translate items")

Signed-off-by: Ori Kam <orika at mellanox.com>
Acked-by: Yongseok Koh <yskoh at mellanox.com>

---

V2:
 * Some cosmetics based on Yongseok comments.

---
 drivers/net/mlx5/mlx5.h         |  2 +-
 drivers/net/mlx5/mlx5_flow.h    |  5 ++-
 drivers/net/mlx5/mlx5_flow_dv.c | 77 +++++++++++++++++++++--------------------
 drivers/net/mlx5/mlx5_rxtx.h    |  7 ----
 4 files changed, 45 insertions(+), 46 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 8de0d74..4122e54 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -208,7 +208,7 @@ struct priv {
 	LIST_HEAD(txqibv, mlx5_txq_ibv) txqsibv; /* Verbs Tx queues. */
 	/* Verbs Indirection tables. */
 	LIST_HEAD(ind_tables, mlx5_ind_table_ibv) ind_tbls;
-	LIST_HEAD(matcher, mlx5_cache) matchers;
+	LIST_HEAD(matchers, mlx5_flow_dv_matcher) matchers;
 	uint32_t link_speed_capa; /* Link speed capabilities. */
 	struct mlx5_xstats_ctrl xstats_ctrl; /* Extended stats control. */
 	int primary_socket; /* Unix socket for primary process. */
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 10d700a..a91f991 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -153,7 +153,10 @@ struct mlx5_flow_dv_match_params {
 
 /* Matcher structure. */
 struct mlx5_flow_dv_matcher {
-	struct mlx5_cache cache; /**< Cache to struct mlx5dv_flow_matcher. */
+	LIST_ENTRY(mlx5_flow_dv_matcher) next;
+	/* Pointer to the next element. */
+	rte_atomic32_t refcnt; /**< Reference counter. */
+	void *matcher_object; /**< Pointer to DV matcher */
 	uint16_t crc; /**< CRC of key. */
 	uint16_t priority; /**< Priority of matcher. */
 	uint8_t egress; /**< Egress matcher. */
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index cf663cd..7d32532 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1060,54 +1060,56 @@
 			 struct rte_flow_error *error)
 {
 	struct priv *priv = dev->data->dev_private;
-	struct mlx5_flow_dv_matcher *cache;
+	struct mlx5_flow_dv_matcher *cache_matcher;
 	struct mlx5dv_flow_matcher_attr dv_attr = {
 		.type = IBV_FLOW_ATTR_NORMAL,
 		.match_mask = (void *)&matcher->mask,
 	};
 
 	/* Lookup from cache. */
-	LIST_FOREACH(cache, &priv->matchers, cache.next) {
-		if (matcher->crc == cache->crc &&
-		    matcher->priority == cache->priority &&
-		    matcher->egress == cache->egress &&
+	LIST_FOREACH(cache_matcher, &priv->matchers, next) {
+		if (matcher->crc == cache_matcher->crc &&
+		    matcher->priority == cache_matcher->priority &&
+		    matcher->egress == cache_matcher->egress &&
 		    !memcmp((const void *)matcher->mask.buf,
-			    (const void *)cache->mask.buf, cache->mask.size)) {
+			    (const void *)cache_matcher->mask.buf,
+			    cache_matcher->mask.size)) {
 			DRV_LOG(DEBUG,
 				"priority %hd use %s matcher %p: refcnt %d++",
-				cache->priority, cache->egress ? "tx" : "rx",
-				(void *)cache,
-				rte_atomic32_read(&cache->cache.refcnt));
-			rte_atomic32_inc(&cache->cache.refcnt);
-			dev_flow->dv.matcher = cache;
+				cache_matcher->priority,
+				cache_matcher->egress ? "tx" : "rx",
+				(void *)cache_matcher,
+				rte_atomic32_read(&cache_matcher->refcnt));
+			rte_atomic32_inc(&cache_matcher->refcnt);
+			dev_flow->dv.matcher = cache_matcher;
 			return 0;
 		}
 	}
 	/* Register new matcher. */
-	cache = rte_calloc(__func__, 1, sizeof(*cache), 0);
-	if (!cache)
+	cache_matcher = rte_calloc(__func__, 1, sizeof(*cache_matcher), 0);
+	if (!cache_matcher)
 		return rte_flow_error_set(error, ENOMEM,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
 					  "cannot allocate matcher memory");
-	*cache = *matcher;
+	*cache_matcher = *matcher;
 	dv_attr.match_criteria_enable =
-		flow_dv_matcher_enable(cache->mask.buf);
+		flow_dv_matcher_enable(cache_matcher->mask.buf);
 	dv_attr.priority = matcher->priority;
 	if (matcher->egress)
 		dv_attr.flags |= IBV_FLOW_ATTR_FLAGS_EGRESS;
-	cache->cache.resource =
+	cache_matcher->matcher_object =
 		mlx5_glue->dv_create_flow_matcher(priv->ctx, &dv_attr);
-	if (!cache->cache.resource)
+	if (!cache_matcher->matcher_object)
 		return rte_flow_error_set(error, ENOMEM,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 					  NULL, "cannot create matcher");
-	rte_atomic32_inc(&cache->cache.refcnt);
-	LIST_INSERT_HEAD(&priv->matchers, &cache->cache, next);
-	dev_flow->dv.matcher = cache;
+	rte_atomic32_inc(&cache_matcher->refcnt);
+	LIST_INSERT_HEAD(&priv->matchers, cache_matcher, next);
+	dev_flow->dv.matcher = cache_matcher;
 	DRV_LOG(DEBUG, "priority %hd new %s matcher %p: refcnt %d",
-		cache->priority,
-		cache->egress ? "tx" : "rx", (void *)cache,
-		rte_atomic32_read(&cache->cache.refcnt));
+		cache_matcher->priority,
+		cache_matcher->egress ? "tx" : "rx", (void *)cache_matcher,
+		rte_atomic32_read(&cache_matcher->refcnt));
 	return 0;
 }
 
@@ -1232,7 +1234,7 @@
 			n++;
 		}
 		dv->flow =
-			mlx5_glue->dv_create_flow(dv->matcher->cache.resource,
+			mlx5_glue->dv_create_flow(dv->matcher->matcher_object,
 						  (void *)&dv->value, n,
 						  dv->actions);
 		if (!dv->flow) {
@@ -1265,28 +1267,29 @@
  *
  * @param dev
  *   Pointer to Ethernet device.
- * @param matcher
- *   Pointer to flow matcher.
+ * @param flow
+ *   Pointer to mlx5_flow.
  *
  * @return
  *   1 while a reference on it exists, 0 when freed.
  */
 static int
 flow_dv_matcher_release(struct rte_eth_dev *dev,
-			struct mlx5_flow_dv_matcher *matcher)
+			struct mlx5_flow *flow)
 {
-	struct mlx5_cache *cache = &matcher->cache;
+	struct mlx5_flow_dv_matcher *matcher = flow->dv.matcher;
 
-	assert(cache->resource);
+	assert(matcher->matcher_object);
 	DRV_LOG(DEBUG, "port %u matcher %p: refcnt %d--",
-		dev->data->port_id, (void *)cache,
-		rte_atomic32_read(&cache->refcnt));
-	if (rte_atomic32_dec_and_test(&cache->refcnt)) {
-		claim_zero(mlx5_glue->dv_destroy_flow_matcher(cache->resource));
-		LIST_REMOVE(cache, next);
-		rte_free(cache);
+		dev->data->port_id, (void *)matcher,
+		rte_atomic32_read(&matcher->refcnt));
+	if (rte_atomic32_dec_and_test(&matcher->refcnt)) {
+		claim_zero(mlx5_glue->dv_destroy_flow_matcher
+			   (matcher->matcher_object));
+		LIST_REMOVE(matcher, next);
+		rte_free(matcher);
 		DRV_LOG(DEBUG, "port %u matcher %p: removed",
-			dev->data->port_id, (void *)cache);
+			dev->data->port_id, (void *)matcher);
 		return 0;
 	}
 	return 1;
@@ -1346,7 +1349,7 @@
 		dev_flow = LIST_FIRST(&flow->dev_flows);
 		LIST_REMOVE(dev_flow, next);
 		if (dev_flow->dv.matcher)
-			flow_dv_matcher_release(dev, dev_flow->dv.matcher);
+			flow_dv_matcher_release(dev, dev_flow);
 		rte_free(dev_flow);
 	}
 }
diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index 02034a4..d225b9c 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -171,13 +171,6 @@ struct mlx5_hrxq {
 	uint8_t rss_key[]; /* Hash key. */
 };
 
-/* List of cached objects. */
-struct mlx5_cache {
-	LIST_ENTRY(mlx5_cache) next; /* Pointer to the next element. */
-	rte_atomic32_t refcnt; /* Reference counter. */
-	void *resource; /* Cached resource */
-};
-
 /* TX queue descriptor. */
 __extension__
 struct mlx5_txq_data {
-- 
1.8.3.1



More information about the dev mailing list