DPDK patches and discussions
 help / color / mirror / Atom feed
From: Gregory Etelson <getelson@nvidia.com>
To: <dev@dpdk.org>
Cc: <getelson@nvidia.com>, <matan@nvidia.com>, <rasland@nvidia.com>,
	Viacheslav Ovsiienko <viacheslavo@nvidia.com>,
	Shahaf Shuler <shahafs@nvidia.com>,
	Xueming Li <xuemingl@nvidia.com>
Subject: [dpdk-dev] [PATCH v5 6/6] net/mlx5: fix crash in tunnel offload setup
Date: Mon, 16 Nov 2020 16:02:23 +0200
Message-ID: <20201116140224.8464-7-getelson@nvidia.com> (raw)
In-Reply-To: <20201116140224.8464-1-getelson@nvidia.com>

The new flow table resource management API triggered a PMD crash in
tunnel offload mode, when tunnel match flow rule was inserted before
tunnel set rule.

Reason for the crash was double flow table registration. The table was
registered by the tunnel offload code for the first time and once
more by PMD code, as part of general table processing. The table
counter was decremented only once during the rule destruction and
caused a resource leak that triggered the crash.

The patch updates PMD registration with tunnel offload parameters and
removes table registration in tunnel related code.

Fixes: 663ad57dabb2 ("net/mlx5: make flow table cache thread safe")

Signed-off-by: Gregory Etelson <getelson@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow.c    |  2 +-
 drivers/net/mlx5/mlx5_flow_dv.c | 39 +++++++++++++++++----------------
 2 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 2fe8648341..b9e1c30713 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -6773,7 +6773,7 @@ mlx5_flow_group_to_table(struct rte_eth_dev *dev,
 		standard_translation = true;
 	}
 	DRV_LOG(DEBUG,
-		"port %u group=%#x transfer=%d external=%d fdb_def_rule=%d translate=%s",
+		"port %u group=%u transfer=%d external=%d fdb_def_rule=%d translate=%s",
 		dev->data->port_id, group, grp_info->transfer,
 		grp_info->external, grp_info->fdb_def_rule,
 		standard_translation ? "STANDARD" : "TUNNEL");
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 25ab9adee6..5e230a3c25 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -8042,6 +8042,8 @@ flow_dv_tbl_resource_get(struct rte_eth_dev *dev,
 				   "cannot get table");
 		return NULL;
 	}
+	DRV_LOG(DEBUG, "Table_id %u tunnel %u group %u registered.",
+		table_id, tunnel ? tunnel->tunnel_id : 0, group_id);
 	tbl_data = container_of(entry, struct mlx5_flow_tbl_data_entry, entry);
 	return &tbl_data->tbl;
 }
@@ -8080,7 +8082,7 @@ flow_dv_tbl_remove_cb(struct mlx5_hlist *list,
 		if (he)
 			mlx5_hlist_unregister(tunnel_grp_hash, he);
 		DRV_LOG(DEBUG,
-			"Table_id %#x tunnel %u group %u released.",
+			"Table_id %u tunnel %u group %u released.",
 			table_id,
 			tbl_data->tunnel ?
 			tbl_data->tunnel->tunnel_id : 0,
@@ -8192,6 +8194,8 @@ flow_dv_matcher_register(struct rte_eth_dev *dev,
 			 struct mlx5_flow_dv_matcher *ref,
 			 union mlx5_flow_tbl_key *key,
 			 struct mlx5_flow *dev_flow,
+			 const struct mlx5_flow_tunnel *tunnel,
+			 uint32_t group_id,
 			 struct rte_flow_error *error)
 {
 	struct mlx5_cache_entry *entry;
@@ -8203,8 +8207,14 @@ flow_dv_matcher_register(struct rte_eth_dev *dev,
 		.data = ref,
 	};
 
-	tbl = flow_dv_tbl_resource_get(dev, key->table_id, key->direction,
-				       key->domain, false, NULL, 0, 0, error);
+	/**
+	 * tunnel offload API requires this registration for cases when
+	 * tunnel match rule was inserted before tunnel set rule.
+	 */
+	tbl = flow_dv_tbl_resource_get(dev, key->table_id,
+				       key->direction, key->domain,
+				       dev_flow->external, tunnel,
+				       group_id, 0, error);
 	if (!tbl)
 		return -rte_errno;	/* No need to refill the error info */
 	tbl_data = container_of(tbl, struct mlx5_flow_tbl_data_entry, tbl);
@@ -9611,10 +9621,14 @@ flow_dv_translate(struct rte_eth_dev *dev,
 		/*
 		 * do not add decap action if match rule drops packet
 		 * HW rejects rules with decap & drop
+		 *
+		 * if tunnel match rule was inserted before matching tunnel set
+		 * rule flow table used in the match rule must be registered.
+		 * current implementation handles that in the
+		 * flow_dv_match_register() at the function end.
 		 */
 		bool add_decap = true;
 		const struct rte_flow_action *ptr = actions;
-		struct mlx5_flow_tbl_resource *tbl;
 
 		for (; ptr->type != RTE_FLOW_ACTION_TYPE_END; ptr++) {
 			if (ptr->type == RTE_FLOW_ACTION_TYPE_DROP) {
@@ -9631,20 +9645,6 @@ flow_dv_translate(struct rte_eth_dev *dev,
 					dev_flow->dv.encap_decap->action;
 			action_flags |= MLX5_FLOW_ACTION_DECAP;
 		}
-		/*
-		 * bind table_id with <group, table> for tunnel match rule.
-		 * Tunnel set rule establishes that bind in JUMP action handler.
-		 * Required for scenario when application creates tunnel match
-		 * rule before tunnel set rule.
-		 */
-		tbl = flow_dv_tbl_resource_get(dev, table, attr->egress,
-					       attr->transfer,
-					       !!dev_flow->external, tunnel,
-					       attr->group, 0, error);
-		if (!tbl)
-			return rte_flow_error_set
-			       (error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
-			       actions, "cannot register tunnel group");
 	}
 	for (; !actions_end ; actions++) {
 		const struct rte_flow_action_queue *queue;
@@ -10474,7 +10474,8 @@ flow_dv_translate(struct rte_eth_dev *dev,
 	tbl_key.domain = attr->transfer;
 	tbl_key.direction = attr->egress;
 	tbl_key.table_id = dev_flow->dv.group;
-	if (flow_dv_matcher_register(dev, &matcher, &tbl_key, dev_flow, error))
+	if (flow_dv_matcher_register(dev, &matcher, &tbl_key, dev_flow,
+				     tunnel, attr->group, error))
 		return -rte_errno;
 	return 0;
 }
-- 
2.29.2


  parent reply	other threads:[~2020-11-16 14:05 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-11  7:14 [dpdk-dev] [PATCH 0/4] restore tunnel offload functionality in mlx5 Gregory Etelson
2020-11-11  7:14 ` [dpdk-dev] [PATCH 1/4] net/mlx5: fix offloaded tunnel allocation Gregory Etelson
2020-11-11  8:51   ` Slava Ovsiienko
2020-11-11  7:14 ` [dpdk-dev] [PATCH 2/4] net/mlx5: fix tunnel offload hub multi-thread protection Gregory Etelson
2020-11-11  8:51   ` Slava Ovsiienko
2020-11-11  7:14 ` [dpdk-dev] [PATCH 3/4] net/mlx5: fix PMD crash after tunnel offload match rule destruction Gregory Etelson
2020-11-11  8:51   ` Slava Ovsiienko
2020-11-11  7:14 ` [dpdk-dev] [PATCH 4/4] net/mlx5: fix tunnel offload callback names Gregory Etelson
2020-11-11  8:51   ` Slava Ovsiienko
2020-11-16  9:13 ` [dpdk-dev] [PATCH v3 0/6] restore tunnel offload functionality in mlx5 Gregory Etelson
2020-11-16  9:13   ` [dpdk-dev] [PATCH v3 1/6] net/mlx5: fix tunnel offload callback names Gregory Etelson
2020-11-16  9:13   ` [dpdk-dev] [PATCH v3 2/6] net/mlx5: fix build with Direct Verbs disabled Gregory Etelson
2020-11-16  9:13   ` [dpdk-dev] [PATCH v3 3/6] net/mlx5: fix structure passing method in function call Gregory Etelson
2020-11-16  9:13   ` [dpdk-dev] [PATCH v3 4/6] net/mlx5: fix tunnel offload object allocation Gregory Etelson
2020-11-16  9:13   ` [dpdk-dev] [PATCH v3 5/6] net/mlx5: fix tunnel offload hub multi-thread protection Gregory Etelson
2020-11-16  9:13   ` [dpdk-dev] [PATCH v3 6/6] net/mlx5: fix crash in tunnel offload setup Gregory Etelson
2020-11-16  9:48 ` [dpdk-dev] [PATCH v4 0/6] restore tunnel offload functionality in mlx5 Gregory Etelson
2020-11-16  9:49   ` [dpdk-dev] [PATCH v4 1/6] net/mlx5: fix tunnel offload callback names Gregory Etelson
2020-11-16  9:49   ` [dpdk-dev] [PATCH v4 2/6] net/mlx5: fix build with Direct Verbs disabled Gregory Etelson
2020-11-16  9:49   ` [dpdk-dev] [PATCH v4 3/6] net/mlx5: fix structure passing method in function call Gregory Etelson
2020-11-16  9:49   ` [dpdk-dev] [PATCH v4 4/6] net/mlx5: fix tunnel offload object allocation Gregory Etelson
2020-11-16  9:49   ` [dpdk-dev] [PATCH v4 5/6] net/mlx5: fix tunnel offload hub multi-thread protection Gregory Etelson
2020-11-16  9:49   ` [dpdk-dev] [PATCH v4 6/6] net/mlx5: fix crash in tunnel offload setup Gregory Etelson
2020-11-16 14:02 ` [dpdk-dev] [PATCH v5 0/6] restore tunnel offload functionality in mlx5 Gregory Etelson
2020-11-16 14:02   ` [dpdk-dev] [PATCH v5 1/6] net/mlx5: fix tunnel offload callback names Gregory Etelson
2020-11-16 14:02   ` [dpdk-dev] [PATCH v5 2/6] net/mlx5: fix build with Direct Verbs disabled Gregory Etelson
2020-11-17 14:33     ` Ferruh Yigit
2020-11-16 14:02   ` [dpdk-dev] [PATCH v5 3/6] net/mlx5: fix structure passing method in function call Gregory Etelson
2020-11-16 14:02   ` [dpdk-dev] [PATCH v5 4/6] net/mlx5: fix tunnel offload object allocation Gregory Etelson
2020-11-16 14:02   ` [dpdk-dev] [PATCH v5 5/6] net/mlx5: fix tunnel offload hub multi-thread protection Gregory Etelson
2020-11-16 14:02   ` Gregory Etelson [this message]
2020-11-17 10:51   ` [dpdk-dev] [PATCH v5 0/6] restore tunnel offload functionality in mlx5 Raslan Darawsheh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201116140224.8464-7-getelson@nvidia.com \
    --to=getelson@nvidia.com \
    --cc=dev@dpdk.org \
    --cc=matan@nvidia.com \
    --cc=rasland@nvidia.com \
    --cc=shahafs@nvidia.com \
    --cc=viacheslavo@nvidia.com \
    --cc=xuemingl@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git