From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 5BB8FA09E0; Fri, 13 Nov 2020 15:54:17 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id ECE77C8DC; Fri, 13 Nov 2020 15:52:56 +0100 (CET) Received: from hqnvemgate26.nvidia.com (hqnvemgate26.nvidia.com [216.228.121.65]) by dpdk.org (Postfix) with ESMTP id B6E0AC8D8 for ; Fri, 13 Nov 2020 15:52:54 +0100 (CET) Received: from hqmail.nvidia.com (Not Verified[216.228.121.13]) by hqnvemgate26.nvidia.com (using TLS: TLSv1.2, AES256-SHA) id ; Fri, 13 Nov 2020 06:52:57 -0800 Received: from nvidia.com (10.124.1.5) by HQMAIL107.nvidia.com (172.20.187.13) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Fri, 13 Nov 2020 14:52:51 +0000 From: Gregory Etelson To: CC: , , , Viacheslav Ovsiienko , Shahaf Shuler , Xueming Li Date: Fri, 13 Nov 2020 16:52:28 +0200 Message-ID: <20201113145231.13154-4-getelson@nvidia.com> X-Mailer: git-send-email 2.29.2 In-Reply-To: <20201113145231.13154-1-getelson@nvidia.com> References: <20201113145231.13154-1-getelson@nvidia.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain X-Originating-IP: [10.124.1.5] X-ClientProxiedBy: HQMAIL105.nvidia.com (172.20.187.12) To HQMAIL107.nvidia.com (172.20.187.13) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1; t=1605279178; bh=lwuDyzZT0PgRko46z205VzccS5E0pXL+kW+7ZoAzGKA=; h=From:To:CC:Subject:Date:Message-ID:X-Mailer:In-Reply-To: References:MIME-Version:Content-Transfer-Encoding:Content-Type: X-Originating-IP:X-ClientProxiedBy; b=XOXSPG++bg+K6wVDn3tUnUaEBl0WA6feLUegQJe9Z/zqWsMXADZ8fw0IeuNPzixFy x2u4pcMVnSYuhvFxOxSGMfQByUoBTKm1YRrM6/LhJ9QUnYjJWvhh3wkEwel6LvkCZW Un1F8Q9FsUl4rvn50ibW9TC+oTNtpZFn+AxnQYhHoRHYwBIfwcZDFHY5bWapxAG1xA b08C6FCtsYY3tCL9doT5qPPQHLakkSROm8ujwoAc/JllzU9bZw2N9cUmYd4zjbBE/j Vx2/IlQOBU5VExMb8I/MESbop2Q2Cke+mc6eUwByuMAB3R3XK0fcM+Q37l9LfQpU1b Nlk2KZwTbxsnw== Subject: [dpdk-dev] [PATCH v2 3/5] net/mlx5: fix double table referencing X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 Acked-by: Viacheslav Ovsiienko --- drivers/net/mlx5/mlx5_flow.c | 16 ++++++++++---- drivers/net/mlx5/mlx5_flow_dv.c | 39 +++++++++++++++++---------------- 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c index 2f01e34033..185b4ba51a 100644 --- a/drivers/net/mlx5/mlx5_flow.c +++ b/drivers/net/mlx5/mlx5_flow.c @@ -7024,7 +7024,15 @@ tunnel_flow_group_to_flow_table(struct rte_eth_dev *= dev, struct mlx5_hlist *group_hash; =20 group_hash =3D tunnel ? tunnel->groups : thub->groups; - he =3D mlx5_hlist_register(group_hash, key.val, NULL); + he =3D mlx5_hlist_lookup(group_hash, key.val, NULL); + if (!he) { + DRV_LOG(DEBUG, "port %u tunnel %u group=3D%u - generate table id", + dev->data->port_id, key.tunnel_id, group); + he =3D mlx5_hlist_register(group_hash, key.val, NULL); + } else { + DRV_LOG(DEBUG, "port %u tunnel %u group=3D%u - skip table id", + dev->data->port_id, key.tunnel_id, group); + } if (!he) return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ATTR_GROUP, @@ -7032,8 +7040,8 @@ tunnel_flow_group_to_flow_table(struct rte_eth_dev *d= ev, "tunnel group index not supported"); tte =3D container_of(he, typeof(*tte), hash); *table =3D tte->flow_table; - DRV_LOG(DEBUG, "port %u tunnel %u group=3D%#x table=3D%#x", - dev->data->port_id, key.tunnel_id, group, *table); + DRV_LOG(DEBUG, "port %u tunnel %u group=3D%u table=3D%u", + dev->data->port_id, key.tunnel_id, group, *table); return 0; } =20 @@ -7114,7 +7122,7 @@ mlx5_flow_group_to_table(struct rte_eth_dev *dev, standard_translation =3D true; } DRV_LOG(DEBUG, - "port %u group=3D%#x transfer=3D%d external=3D%d fdb_def_rule=3D%d trans= late=3D%s", + "port %u group=3D%u transfer=3D%d external=3D%d fdb_def_rule=3D%d transl= ate=3D%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_d= v.c index 78c710fef9..95165980f4 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 =3D 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 =3D ref, }; =20 - tbl =3D 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 =3D 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 =3D container_of(tbl, struct mlx5_flow_tbl_data_entry, tbl); @@ -9605,10 +9615,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 =3D true; const struct rte_flow_action *ptr =3D actions; - struct mlx5_flow_tbl_resource *tbl; =20 for (; ptr->type !=3D RTE_FLOW_ACTION_TYPE_END; ptr++) { if (ptr->type =3D=3D RTE_FLOW_ACTION_TYPE_DROP) { @@ -9625,20 +9639,6 @@ flow_dv_translate(struct rte_eth_dev *dev, dev_flow->dv.encap_decap->action; action_flags |=3D MLX5_FLOW_ACTION_DECAP; } - /* - * bind table_id with 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 =3D 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; @@ -10468,7 +10468,8 @@ flow_dv_translate(struct rte_eth_dev *dev, tbl_key.domain =3D attr->transfer; tbl_key.direction =3D attr->egress; tbl_key.table_id =3D 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; } --=20 2.29.2