[PATCH 01/20] net/mlx5: fix wrong check sibling device config mismatch

[PATCH 01/20] net/mlx5: fix wrong check sibling device config mismatch

[Michael Baum]

The MLX5 net driver supports "probe again". In probing again, it
creates a new ethdev under an existing infiniband device context.

Sibling devices sharing infiniband device context should have compatible
configurations, so some of the devargs given in the probe again, the
ones that are mainly relevant to the sharing device context are sent to
the mlx5_dev_check_sibling_config function which makes sure that they
compatible its siblings.
However, the arguments are adjusted according to the capability of the
device, and the function compares the arguments of the probe again
before the adjustment with the arguments of the siblings after the
adjustment. A user who sends the same values to all siblings may fail in
this comparison if he requested something that the device does not
support and adjusted.

This patch moves the call to the mlx5_dev_check_sibling_config function
after the relevant adjustments.

Fixes: 92d5dd483450 ("net/mlx5: check sibling device configurations mismatch")
Fixes: 2d241515ebaf ("net/mlx5: add devarg for extensive metadata support")
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/linux/mlx5_os.c   | 41 ++++++++++++++++--------------
 drivers/net/mlx5/windows/mlx5_os.c | 28 ++++++++++++--------
 2 files changed, 39 insertions(+), 30 deletions(-)

diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index aecdc5a68a..de0bb87460 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -1241,6 +1241,28 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 	}
 	/* Override some values set by hardware configuration. */
 	mlx5_args(config, dpdk_dev->devargs);
+	/* Update final values for devargs before check sibling config. */
+#if !defined(HAVE_IBV_FLOW_DV_SUPPORT) || !defined(HAVE_MLX5DV_DR)
+	if (config->dv_flow_en) {
+		DRV_LOG(WARNING, "DV flow is not supported.");
+		config->dv_flow_en = 0;
+	}
+#endif
+#ifdef HAVE_MLX5DV_DR_ESWITCH
+	if (!(sh->cdev->config.hca_attr.eswitch_manager && config->dv_flow_en &&
+	      (switch_info->representor || switch_info->master)))
+		config->dv_esw_en = 0;
+#else
+	config->dv_esw_en = 0;
+#endif
+	if (!priv->config.dv_esw_en &&
+	    priv->config.dv_xmeta_en != MLX5_XMETA_MODE_LEGACY) {
+		DRV_LOG(WARNING,
+			"Metadata mode %u is not supported (no E-Switch).",
+			priv->config.dv_xmeta_en);
+		priv->config.dv_xmeta_en = MLX5_XMETA_MODE_LEGACY;
+	}
+	/* Check sibling device configurations. */
 	err = mlx5_dev_check_sibling_config(priv, config, dpdk_dev);
 	if (err)
 		goto error;
@@ -1251,12 +1273,6 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 #if !defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42) && \
 	!defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)
 	DRV_LOG(DEBUG, "counters are not supported");
-#endif
-#if !defined(HAVE_IBV_FLOW_DV_SUPPORT) || !defined(HAVE_MLX5DV_DR)
-	if (config->dv_flow_en) {
-		DRV_LOG(WARNING, "DV flow is not supported");
-		config->dv_flow_en = 0;
-	}
 #endif
 	config->ind_table_max_size =
 		sh->device_attr.max_rwq_indirection_table_size;
@@ -1652,13 +1668,6 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 	 * Verbs context returned by ibv_open_device().
 	 */
 	mlx5_link_update(eth_dev, 0);
-#ifdef HAVE_MLX5DV_DR_ESWITCH
-	if (!(config->hca_attr.eswitch_manager && config->dv_flow_en &&
-	      (switch_info->representor || switch_info->master)))
-		config->dv_esw_en = 0;
-#else
-	config->dv_esw_en = 0;
-#endif
 	/* Detect minimal data bytes to inline. */
 	mlx5_set_min_inline(spawn, config);
 	/* Store device configuration on private structure. */
@@ -1725,12 +1734,6 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 		err = -err;
 		goto error;
 	}
-	if (!priv->config.dv_esw_en &&
-	    priv->config.dv_xmeta_en != MLX5_XMETA_MODE_LEGACY) {
-		DRV_LOG(WARNING, "metadata mode %u is not supported "
-				 "(no E-Switch)", priv->config.dv_xmeta_en);
-		priv->config.dv_xmeta_en = MLX5_XMETA_MODE_LEGACY;
-	}
 	mlx5_set_metadata_mask(eth_dev);
 	if (priv->config.dv_xmeta_en != MLX5_XMETA_MODE_LEGACY &&
 	    !priv->sh->dv_regc0_mask) {
diff --git a/drivers/net/mlx5/windows/mlx5_os.c b/drivers/net/mlx5/windows/mlx5_os.c
index ac0af0ff7d..8eb53f2cb7 100644
--- a/drivers/net/mlx5/windows/mlx5_os.c
+++ b/drivers/net/mlx5/windows/mlx5_os.c
@@ -439,6 +439,21 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 	}
 	/* Override some values set by hardware configuration. */
 	mlx5_args(config, dpdk_dev->devargs);
+	/* Update final values for devargs before check sibling config. */
+	config->dv_esw_en = 0;
+	if (!config->dv_flow_en) {
+		DRV_LOG(ERR, "Windows flow mode must be DV flow enable.");
+		err = ENOTSUP;
+		goto error;
+	}
+	if (!priv->config.dv_esw_en &&
+	    priv->config.dv_xmeta_en != MLX5_XMETA_MODE_LEGACY) {
+		DRV_LOG(WARNING,
+			"Metadata mode %u is not supported (no E-Switch).",
+			priv->config.dv_xmeta_en);
+		priv->config.dv_xmeta_en = MLX5_XMETA_MODE_LEGACY;
+	}
+	/* Check sibling device configurations. */
 	err = mlx5_dev_check_sibling_config(priv, config, dpdk_dev);
 	if (err)
 		goto error;
@@ -600,7 +615,6 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 	 * Verbs context returned by ibv_open_device().
 	 */
 	mlx5_link_update(eth_dev, 0);
-	config->dv_esw_en = 0;
 	/* Detect minimal data bytes to inline. */
 	mlx5_set_min_inline(spawn, config);
 	/* Store device configuration on private structure. */
@@ -622,12 +636,6 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 	}
 	/* No supported flow priority number detection. */
 	priv->sh->flow_max_priority = -1;
-	if (!priv->config.dv_esw_en &&
-	    priv->config.dv_xmeta_en != MLX5_XMETA_MODE_LEGACY) {
-		DRV_LOG(WARNING, "metadata mode %u is not supported "
-				 "(no E-Switch)", priv->config.dv_xmeta_en);
-		priv->config.dv_xmeta_en = MLX5_XMETA_MODE_LEGACY;
-	}
 	mlx5_set_metadata_mask(eth_dev);
 	if (priv->config.dv_xmeta_en != MLX5_XMETA_MODE_LEGACY &&
 	    !priv->sh->dv_regc0_mask) {
@@ -661,12 +669,10 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 			goto error;
 		}
 	}
-	if (sh->devx && config->dv_flow_en) {
+	if (sh->devx) {
 		priv->obj_ops = devx_obj_ops;
 	} else {
-		DRV_LOG(ERR, "Flow mode %u is not supported "
-				"(Windows flow must be DevX with DV flow enabled).",
-				priv->config.dv_flow_en);
+		DRV_LOG(ERR, "Windows flow must be DevX.");
 		err = ENOTSUP;
 		goto error;
 	}
-- 
2.25.1

[PATCH 02/20] net/mlx5: fix ineffective metadata argument adjustment

[Michael Baum]

In "dv_xmeta_en" devarg there is an option of dv_xmeta_en=3 which
engages tunnel offload mode. In E-Switch configuration, that mode
implicitly activates dv_xmeta_en=1.

The update according to E-switch support is done immediately after the
first parsing of the devargs, but there is another adjustment later.

This patch moves the adjustment after the second parsing.

Fixes: 4ec6360de37d ("net/mlx5: implement tunnel offload")
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/linux/mlx5_os.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index de0bb87460..e45e56f4b6 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -977,10 +977,6 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 			strerror(rte_errno));
 		goto error;
 	}
-	if (config->dv_miss_info) {
-		if (switch_info->master || switch_info->representor)
-			config->dv_xmeta_en = MLX5_XMETA_MODE_META16;
-	}
 	sh = mlx5_alloc_shared_dev_ctx(spawn, config);
 	if (!sh)
 		return NULL;
@@ -1242,6 +1238,10 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 	/* Override some values set by hardware configuration. */
 	mlx5_args(config, dpdk_dev->devargs);
 	/* Update final values for devargs before check sibling config. */
+	if (config->dv_miss_info) {
+		if (switch_info->master || switch_info->representor)
+			config->dv_xmeta_en = MLX5_XMETA_MODE_META16;
+	}
 #if !defined(HAVE_IBV_FLOW_DV_SUPPORT) || !defined(HAVE_MLX5DV_DR)
 	if (config->dv_flow_en) {
 		DRV_LOG(WARNING, "DV flow is not supported.");
-- 
2.25.1

[PATCH 03/20] net/mlx5: fix wrong place of ASO CT object release

[Michael Baum]

The ASO connection tracking structure is initialized once for sharing
device context.

Its release takes place in the close function which is called for each
ethdev individually. i.e. when there is more than one ethdev under the
same sharing device context, it will be destroyed when one of them is
closed. If the other wants to use it later, it may cause it to crash.

In addition, the creation of this structure is performed in the spawn
function. if one of the creations of the objects following it fails, it
is supposed to be destroyed but this does not happen.

This patch moves its release to the sharing device context free function
and thus solves both problems.

Fixes: 0af8a2298a42 ("net/mlx5: release connection tracking management")
Fixes: ee9e5fad03eb ("net/mlx5: initialize connection tracking management")
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/mlx5.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 67eda41a60..d1d398f49a 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -1321,6 +1321,8 @@ mlx5_free_shared_dev_ctx(struct mlx5_dev_ctx_shared *sh)
 	 *  Only primary process handles async device events.
 	 **/
 	mlx5_flow_counters_mng_close(sh);
+	if (sh->ct_mng)
+		mlx5_flow_aso_ct_mng_close(sh);
 	if (sh->aso_age_mng) {
 		mlx5_flow_aso_age_mng_close(sh);
 		sh->aso_age_mng = NULL;
@@ -1594,8 +1596,6 @@ mlx5_dev_close(struct rte_eth_dev *dev)
 	if (priv->mreg_cp_tbl)
 		mlx5_hlist_destroy(priv->mreg_cp_tbl);
 	mlx5_mprq_free_mp(dev);
-	if (priv->sh->ct_mng)
-		mlx5_flow_aso_ct_mng_close(priv->sh);
 	mlx5_os_free_shared_dr(priv);
 	if (priv->rss_conf.rss_key != NULL)
 		mlx5_free(priv->rss_conf.rss_key);
-- 
2.25.1

[PATCH 04/20] net/mlx5: fix inconsistency errno update in SH creation

[Michael Baum]

The mlx5_alloc_shared_dev_ctx() function has a local variable named
"err" which contains the errno value in case of failure.

When functions called by this function are failed, this variable is
updated with their return value (that should be a positive errno value).
However, some functions doesn't update errno value by themselves or
return negative errno value. If one of them fails, the "err" variable
contains negative value what cause to assertion failure.

This patch updates all functions uses by mlx5_alloc_shared_dev_ctx()
function to update rte_errno and take this value instead of "err" value.

Fixes: 5dfa003db53f ("common/mlx5: fix post doorbell barrier")
Fixes: 5d55a494f4e6 ("net/mlx5: split multi-thread flow handling per OS")
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/linux/mlx5_flow_os.c   |  3 ++-
 drivers/net/mlx5/linux/mlx5_os.c        | 16 ++++++++++------
 drivers/net/mlx5/mlx5.c                 | 24 +++++++++++-------------
 drivers/net/mlx5/windows/mlx5_flow_os.c |  2 +-
 drivers/net/mlx5/windows/mlx5_os.c      | 24 ++++++++++++------------
 5 files changed, 36 insertions(+), 33 deletions(-)

diff --git a/drivers/net/mlx5/linux/mlx5_flow_os.c b/drivers/net/mlx5/linux/mlx5_flow_os.c
index 893f00b824..a5956c255a 100644
--- a/drivers/net/mlx5/linux/mlx5_flow_os.c
+++ b/drivers/net/mlx5/linux/mlx5_flow_os.c
@@ -14,7 +14,8 @@ mlx5_flow_os_init_workspace_once(void)
 {
 	if (rte_thread_key_create(&key_workspace, flow_release_workspace)) {
 		DRV_LOG(ERR, "Can't create flow workspace data thread key.");
-		return -ENOMEM;
+		rte_errno = ENOMEM;
+		return -rte_errno;
 	}
 	return 0;
 }
diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index e45e56f4b6..f587a47f6e 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -138,7 +138,7 @@ mlx5_os_set_nonblock_channel_fd(int fd)
  *   Pointer to mlx5 device attributes.
  *
  * @return
- *   0 on success, non zero error number otherwise
+ *   0 on success, a negative errno value otherwise and rte_errno is set.
  */
 int
 mlx5_os_get_dev_attr(struct mlx5_common_device *cdev,
@@ -150,8 +150,10 @@ mlx5_os_get_dev_attr(struct mlx5_common_device *cdev,
 
 	memset(device_attr, 0, sizeof(*device_attr));
 	err = mlx5_glue->query_device_ex(ctx, NULL, &attr_ex);
-	if (err)
-		return err;
+	if (err) {
+		rte_errno = errno;
+		return -rte_errno;
+	}
 	device_attr->device_cap_flags_ex = attr_ex.device_cap_flags_ex;
 	device_attr->max_qp_wr = attr_ex.orig_attr.max_qp_wr;
 	device_attr->max_sge = attr_ex.orig_attr.max_sge;
@@ -170,8 +172,10 @@ mlx5_os_get_dev_attr(struct mlx5_common_device *cdev,
 
 	struct mlx5dv_context dv_attr = { .comp_mask = 0 };
 	err = mlx5_glue->dv_query_device(ctx, &dv_attr);
-	if (err)
-		return err;
+	if (err) {
+		rte_errno = errno;
+		return -rte_errno;
+	}
 
 	device_attr->flags = dv_attr.flags;
 	device_attr->comp_mask = dv_attr.comp_mask;
@@ -195,7 +199,7 @@ mlx5_os_get_dev_attr(struct mlx5_common_device *cdev,
 	strlcpy(device_attr->fw_ver, attr_ex.orig_attr.fw_ver,
 		sizeof(device_attr->fw_ver));
 
-	return err;
+	return 0;
 }
 
 /**
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index d1d398f49a..25d4d2082b 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -1172,12 +1172,11 @@ mlx5_alloc_shared_dev_ctx(const struct mlx5_dev_spawn_data *spawn,
 	MLX5_ASSERT(spawn->max_port);
 	sh = mlx5_malloc(MLX5_MEM_ZERO | MLX5_MEM_RTE,
 			 sizeof(struct mlx5_dev_ctx_shared) +
-			 spawn->max_port *
-			 sizeof(struct mlx5_dev_shared_port),
+			 spawn->max_port * sizeof(struct mlx5_dev_shared_port),
 			 RTE_CACHE_LINE_SIZE, SOCKET_ID_ANY);
 	if (!sh) {
-		DRV_LOG(ERR, "shared context allocation failure");
-		rte_errno  = ENOMEM;
+		DRV_LOG(ERR, "Shared context allocation failure.");
+		rte_errno = ENOMEM;
 		goto exit;
 	}
 	pthread_mutex_init(&sh->txpp.mutex, NULL);
@@ -1199,9 +1198,8 @@ mlx5_alloc_shared_dev_ctx(const struct mlx5_dev_spawn_data *spawn,
 	strncpy(sh->ibdev_path, mlx5_os_get_ctx_device_path(sh->cdev->ctx),
 		sizeof(sh->ibdev_path) - 1);
 	/*
-	 * Setting port_id to max unallowed value means
-	 * there is no interrupt subhandler installed for
-	 * the given port index i.
+	 * Setting port_id to max unallowed value means there is no interrupt
+	 * subhandler installed for the given port index i.
 	 */
 	for (i = 0; i < sh->max_port; i++) {
 		sh->port[i].ih_port_id = RTE_MAX_ETHPORTS;
@@ -1211,12 +1209,12 @@ mlx5_alloc_shared_dev_ctx(const struct mlx5_dev_spawn_data *spawn,
 		sh->td = mlx5_devx_cmd_create_td(sh->cdev->ctx);
 		if (!sh->td) {
 			DRV_LOG(ERR, "TD allocation failure");
-			err = ENOMEM;
+			rte_errno = ENOMEM;
 			goto error;
 		}
 		if (mlx5_setup_tis(sh)) {
 			DRV_LOG(ERR, "TIS allocation failure");
-			err = ENOMEM;
+			rte_errno = ENOMEM;
 			goto error;
 		}
 		err = mlx5_rxtx_uars_prepare(sh);
@@ -1246,19 +1244,19 @@ mlx5_alloc_shared_dev_ctx(const struct mlx5_dev_spawn_data *spawn,
 	pthread_mutex_unlock(&mlx5_dev_ctx_list_mutex);
 	return sh;
 error:
+	err = rte_errno;
 	pthread_mutex_destroy(&sh->txpp.mutex);
 	pthread_mutex_unlock(&mlx5_dev_ctx_list_mutex);
 	MLX5_ASSERT(sh);
-	if (sh->td)
-		claim_zero(mlx5_devx_cmd_destroy(sh->td));
+	mlx5_rxtx_uars_release(sh);
 	i = 0;
 	do {
 		if (sh->tis[i])
 			claim_zero(mlx5_devx_cmd_destroy(sh->tis[i]));
 	} while (++i < (uint32_t)sh->bond.n_port);
-	mlx5_rxtx_uars_release(sh);
+	if (sh->td)
+		claim_zero(mlx5_devx_cmd_destroy(sh->td));
 	mlx5_free(sh);
-	MLX5_ASSERT(err > 0);
 	rte_errno = err;
 	return NULL;
 }
diff --git a/drivers/net/mlx5/windows/mlx5_flow_os.c b/drivers/net/mlx5/windows/mlx5_flow_os.c
index 7bb4c4590a..f5e3893ed4 100644
--- a/drivers/net/mlx5/windows/mlx5_flow_os.c
+++ b/drivers/net/mlx5/windows/mlx5_flow_os.c
@@ -372,7 +372,7 @@ mlx5_flow_os_init_workspace_once(void)
 
 	if (err) {
 		DRV_LOG(ERR, "Can't create flow workspace data thread key.");
-		return err;
+		return -rte_errno;
 	}
 	pthread_mutex_init(&lock_thread_list, NULL);
 	return 0;
diff --git a/drivers/net/mlx5/windows/mlx5_os.c b/drivers/net/mlx5/windows/mlx5_os.c
index 8eb53f2cb7..d31de6a5e8 100644
--- a/drivers/net/mlx5/windows/mlx5_os.c
+++ b/drivers/net/mlx5/windows/mlx5_os.c
@@ -152,7 +152,7 @@ mlx5_init_once(void)
  *   Pointer to mlx5 device attributes.
  *
  * @return
- *   0 on success, non zero error number otherwise.
+ *   0 on success, a negative errno value otherwise and rte_errno is set.
  */
 int
 mlx5_os_get_dev_attr(struct mlx5_common_device *cdev,
@@ -161,10 +161,11 @@ mlx5_os_get_dev_attr(struct mlx5_common_device *cdev,
 	struct mlx5_context *mlx5_ctx;
 	void *pv_iseg = NULL;
 	u32 cb_iseg = 0;
-	int err = 0;
 
-	if (!cdev || !cdev->ctx)
-		return -EINVAL;
+	if (!cdev || !cdev->ctx) {
+		rte_errno = EINVAL;
+		return -rte_errno;
+	}
 	mlx5_ctx = (struct mlx5_context *)cdev->ctx;
 	memset(device_attr, 0, sizeof(*device_attr));
 	device_attr->max_cq = 1 << cdev->config.hca_attr.log_max_cq;
@@ -187,15 +188,14 @@ mlx5_os_get_dev_attr(struct mlx5_common_device *cdev,
 	pv_iseg = mlx5_glue->query_hca_iseg(mlx5_ctx, &cb_iseg);
 	if (pv_iseg == NULL) {
 		DRV_LOG(ERR, "Failed to get device hca_iseg");
-		return errno;
-	}
-	if (!err) {
-		snprintf(device_attr->fw_ver, 64, "%x.%x.%04x",
-			MLX5_GET(initial_seg, pv_iseg, fw_rev_major),
-			MLX5_GET(initial_seg, pv_iseg, fw_rev_minor),
-			MLX5_GET(initial_seg, pv_iseg, fw_rev_subminor));
+		rte_errno = errno;
+		return -rte_errno;
 	}
-	return err;
+	snprintf(device_attr->fw_ver, 64, "%x.%x.%04x",
+		 MLX5_GET(initial_seg, pv_iseg, fw_rev_major),
+		 MLX5_GET(initial_seg, pv_iseg, fw_rev_minor),
+		 MLX5_GET(initial_seg, pv_iseg, fw_rev_subminor));
+	return 0;
 }
 
 /**
-- 
2.25.1

[PATCH v2 01/20] net/mlx5: fix wrong check sibling device config mismatch

[Michael Baum]

The MLX5 net driver supports "probe again". In probing again, it
creates a new ethdev under an existing infiniband device context.

Sibling devices sharing infiniband device context should have compatible
configurations, so some of the devargs given in the probe again, the
ones that are mainly relevant to the sharing device context are sent to
the mlx5_dev_check_sibling_config function which makes sure that they
compatible its siblings.
However, the arguments are adjusted according to the capability of the
device, and the function compares the arguments of the probe again
before the adjustment with the arguments of the siblings after the
adjustment. A user who sends the same values to all siblings may fail in
this comparison if he requested something that the device does not
support and adjusted.

This patch moves the call to the mlx5_dev_check_sibling_config function
after the relevant adjustments.

Fixes: 92d5dd483450 ("net/mlx5: check sibling device configurations mismatch")
Fixes: 2d241515ebaf ("net/mlx5: add devarg for extensive metadata support")
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/linux/mlx5_os.c   | 41 ++++++++++++++++--------------
 drivers/net/mlx5/windows/mlx5_os.c | 28 ++++++++++++--------
 2 files changed, 39 insertions(+), 30 deletions(-)

diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index bbe05bb837..e157795b63 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -1241,6 +1241,28 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 	}
 	/* Override some values set by hardware configuration. */
 	mlx5_args(config, dpdk_dev->devargs);
+	/* Update final values for devargs before check sibling config. */
+#if !defined(HAVE_IBV_FLOW_DV_SUPPORT) || !defined(HAVE_MLX5DV_DR)
+	if (config->dv_flow_en) {
+		DRV_LOG(WARNING, "DV flow is not supported.");
+		config->dv_flow_en = 0;
+	}
+#endif
+#ifdef HAVE_MLX5DV_DR_ESWITCH
+	if (!(sh->cdev->config.hca_attr.eswitch_manager && config->dv_flow_en &&
+	      (switch_info->representor || switch_info->master)))
+		config->dv_esw_en = 0;
+#else
+	config->dv_esw_en = 0;
+#endif
+	if (!priv->config.dv_esw_en &&
+	    priv->config.dv_xmeta_en != MLX5_XMETA_MODE_LEGACY) {
+		DRV_LOG(WARNING,
+			"Metadata mode %u is not supported (no E-Switch).",
+			priv->config.dv_xmeta_en);
+		priv->config.dv_xmeta_en = MLX5_XMETA_MODE_LEGACY;
+	}
+	/* Check sibling device configurations. */
 	err = mlx5_dev_check_sibling_config(priv, config, dpdk_dev);
 	if (err)
 		goto error;
@@ -1251,12 +1273,6 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 #if !defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42) && \
 	!defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)
 	DRV_LOG(DEBUG, "counters are not supported");
-#endif
-#if !defined(HAVE_IBV_FLOW_DV_SUPPORT) || !defined(HAVE_MLX5DV_DR)
-	if (config->dv_flow_en) {
-		DRV_LOG(WARNING, "DV flow is not supported");
-		config->dv_flow_en = 0;
-	}
 #endif
 	config->ind_table_max_size =
 		sh->device_attr.max_rwq_indirection_table_size;
@@ -1652,13 +1668,6 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 	 * Verbs context returned by ibv_open_device().
 	 */
 	mlx5_link_update(eth_dev, 0);
-#ifdef HAVE_MLX5DV_DR_ESWITCH
-	if (!(config->hca_attr.eswitch_manager && config->dv_flow_en &&
-	      (switch_info->representor || switch_info->master)))
-		config->dv_esw_en = 0;
-#else
-	config->dv_esw_en = 0;
-#endif
 	/* Detect minimal data bytes to inline. */
 	mlx5_set_min_inline(spawn, config);
 	/* Store device configuration on private structure. */
@@ -1725,12 +1734,6 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 		err = -err;
 		goto error;
 	}
-	if (!priv->config.dv_esw_en &&
-	    priv->config.dv_xmeta_en != MLX5_XMETA_MODE_LEGACY) {
-		DRV_LOG(WARNING, "metadata mode %u is not supported "
-				 "(no E-Switch)", priv->config.dv_xmeta_en);
-		priv->config.dv_xmeta_en = MLX5_XMETA_MODE_LEGACY;
-	}
 	mlx5_set_metadata_mask(eth_dev);
 	if (priv->config.dv_xmeta_en != MLX5_XMETA_MODE_LEGACY &&
 	    !priv->sh->dv_regc0_mask) {
diff --git a/drivers/net/mlx5/windows/mlx5_os.c b/drivers/net/mlx5/windows/mlx5_os.c
index 7f3532426f..3db33cd0cf 100644
--- a/drivers/net/mlx5/windows/mlx5_os.c
+++ b/drivers/net/mlx5/windows/mlx5_os.c
@@ -439,6 +439,21 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 	}
 	/* Override some values set by hardware configuration. */
 	mlx5_args(config, dpdk_dev->devargs);
+	/* Update final values for devargs before check sibling config. */
+	config->dv_esw_en = 0;
+	if (!config->dv_flow_en) {
+		DRV_LOG(ERR, "Windows flow mode must be DV flow enable.");
+		err = ENOTSUP;
+		goto error;
+	}
+	if (!priv->config.dv_esw_en &&
+	    priv->config.dv_xmeta_en != MLX5_XMETA_MODE_LEGACY) {
+		DRV_LOG(WARNING,
+			"Metadata mode %u is not supported (no E-Switch).",
+			priv->config.dv_xmeta_en);
+		priv->config.dv_xmeta_en = MLX5_XMETA_MODE_LEGACY;
+	}
+	/* Check sibling device configurations. */
 	err = mlx5_dev_check_sibling_config(priv, config, dpdk_dev);
 	if (err)
 		goto error;
@@ -600,7 +615,6 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 	 * Verbs context returned by ibv_open_device().
 	 */
 	mlx5_link_update(eth_dev, 0);
-	config->dv_esw_en = 0;
 	/* Detect minimal data bytes to inline. */
 	mlx5_set_min_inline(spawn, config);
 	/* Store device configuration on private structure. */
@@ -622,12 +636,6 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 	}
 	/* No supported flow priority number detection. */
 	priv->sh->flow_max_priority = -1;
-	if (!priv->config.dv_esw_en &&
-	    priv->config.dv_xmeta_en != MLX5_XMETA_MODE_LEGACY) {
-		DRV_LOG(WARNING, "metadata mode %u is not supported "
-				 "(no E-Switch)", priv->config.dv_xmeta_en);
-		priv->config.dv_xmeta_en = MLX5_XMETA_MODE_LEGACY;
-	}
 	mlx5_set_metadata_mask(eth_dev);
 	if (priv->config.dv_xmeta_en != MLX5_XMETA_MODE_LEGACY &&
 	    !priv->sh->dv_regc0_mask) {
@@ -661,12 +669,10 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 			goto error;
 		}
 	}
-	if (sh->devx && config->dv_flow_en) {
+	if (sh->devx) {
 		priv->obj_ops = devx_obj_ops;
 	} else {
-		DRV_LOG(ERR, "Flow mode %u is not supported "
-				"(Windows flow must be DevX with DV flow enabled).",
-				priv->config.dv_flow_en);
+		DRV_LOG(ERR, "Windows flow must be DevX.");
 		err = ENOTSUP;
 		goto error;
 	}
-- 
2.25.1

[PATCH v2 02/20] net/mlx5: fix ineffective metadata argument adjustment

[Michael Baum]

In "dv_xmeta_en" devarg there is an option of dv_xmeta_en=3 which
engages tunnel offload mode. In E-Switch configuration, that mode
implicitly activates dv_xmeta_en=1.

The update according to E-switch support is done immediately after the
first parsing of the devargs, but there is another adjustment later.

This patch moves the adjustment after the second parsing.

Fixes: 4ec6360de37d ("net/mlx5: implement tunnel offload")
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/linux/mlx5_os.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index e157795b63..69d3e1e3ad 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -977,10 +977,6 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 			strerror(rte_errno));
 		goto error;
 	}
-	if (config->dv_miss_info) {
-		if (switch_info->master || switch_info->representor)
-			config->dv_xmeta_en = MLX5_XMETA_MODE_META16;
-	}
 	sh = mlx5_alloc_shared_dev_ctx(spawn, config);
 	if (!sh)
 		return NULL;
@@ -1242,6 +1238,10 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
 	/* Override some values set by hardware configuration. */
 	mlx5_args(config, dpdk_dev->devargs);
 	/* Update final values for devargs before check sibling config. */
+	if (config->dv_miss_info) {
+		if (switch_info->master || switch_info->representor)
+			config->dv_xmeta_en = MLX5_XMETA_MODE_META16;
+	}
 #if !defined(HAVE_IBV_FLOW_DV_SUPPORT) || !defined(HAVE_MLX5DV_DR)
 	if (config->dv_flow_en) {
 		DRV_LOG(WARNING, "DV flow is not supported.");
-- 
2.25.1

[PATCH v2 03/20] net/mlx5: fix wrong place of ASO CT object release

[Michael Baum]

The ASO connection tracking structure is initialized once for sharing
device context.

Its release takes place in the close function which is called for each
ethdev individually. i.e. when there is more than one ethdev under the
same sharing device context, it will be destroyed when one of them is
closed. If the other wants to use it later, it may cause it to crash.

In addition, the creation of this structure is performed in the spawn
function. if one of the creations of the objects following it fails, it
is supposed to be destroyed but this does not happen.

This patch moves its release to the sharing device context free function
and thus solves both problems.

Fixes: 0af8a2298a42 ("net/mlx5: release connection tracking management")
Fixes: ee9e5fad03eb ("net/mlx5: initialize connection tracking management")
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/mlx5.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 5571e90677..cde8d022cd 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -1321,6 +1321,8 @@ mlx5_free_shared_dev_ctx(struct mlx5_dev_ctx_shared *sh)
 	 *  Only primary process handles async device events.
 	 **/
 	mlx5_flow_counters_mng_close(sh);
+	if (sh->ct_mng)
+		mlx5_flow_aso_ct_mng_close(sh);
 	if (sh->aso_age_mng) {
 		mlx5_flow_aso_age_mng_close(sh);
 		sh->aso_age_mng = NULL;
@@ -1594,8 +1596,6 @@ mlx5_dev_close(struct rte_eth_dev *dev)
 	if (priv->mreg_cp_tbl)
 		mlx5_hlist_destroy(priv->mreg_cp_tbl);
 	mlx5_mprq_free_mp(dev);
-	if (priv->sh->ct_mng)
-		mlx5_flow_aso_ct_mng_close(priv->sh);
 	mlx5_os_free_shared_dr(priv);
 	if (priv->rss_conf.rss_key != NULL)
 		mlx5_free(priv->rss_conf.rss_key);
-- 
2.25.1

[PATCH v2 04/20] net/mlx5: fix inconsistency errno update in SH creation

[Michael Baum]

The mlx5_alloc_shared_dev_ctx() function has a local variable named
"err" which contains the errno value in case of failure.

When functions called by this function are failed, this variable is
updated with their return value (that should be a positive errno value).
However, some functions doesn't update errno value by themselves or
return negative errno value. If one of them fails, the "err" variable
contains negative value what cause to assertion failure.

This patch updates all functions uses by mlx5_alloc_shared_dev_ctx()
function to update rte_errno and take this value instead of "err" value.

Fixes: 5dfa003db53f ("common/mlx5: fix post doorbell barrier")
Fixes: 5d55a494f4e6 ("net/mlx5: split multi-thread flow handling per OS")
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/linux/mlx5_flow_os.c   |  3 ++-
 drivers/net/mlx5/linux/mlx5_os.c        | 16 ++++++++++------
 drivers/net/mlx5/mlx5.c                 | 24 +++++++++++-------------
 drivers/net/mlx5/windows/mlx5_flow_os.c |  2 +-
 drivers/net/mlx5/windows/mlx5_os.c      | 24 ++++++++++++------------
 5 files changed, 36 insertions(+), 33 deletions(-)

diff --git a/drivers/net/mlx5/linux/mlx5_flow_os.c b/drivers/net/mlx5/linux/mlx5_flow_os.c
index 893f00b824..a5956c255a 100644
--- a/drivers/net/mlx5/linux/mlx5_flow_os.c
+++ b/drivers/net/mlx5/linux/mlx5_flow_os.c
@@ -14,7 +14,8 @@ mlx5_flow_os_init_workspace_once(void)
 {
 	if (rte_thread_key_create(&key_workspace, flow_release_workspace)) {
 		DRV_LOG(ERR, "Can't create flow workspace data thread key.");
-		return -ENOMEM;
+		rte_errno = ENOMEM;
+		return -rte_errno;
 	}
 	return 0;
 }
diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index 69d3e1e3ad..faab310b11 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -138,7 +138,7 @@ mlx5_os_set_nonblock_channel_fd(int fd)
  *   Pointer to mlx5 device attributes.
  *
  * @return
- *   0 on success, non zero error number otherwise
+ *   0 on success, a negative errno value otherwise and rte_errno is set.
  */
 int
 mlx5_os_get_dev_attr(struct mlx5_common_device *cdev,
@@ -150,8 +150,10 @@ mlx5_os_get_dev_attr(struct mlx5_common_device *cdev,
 
 	memset(device_attr, 0, sizeof(*device_attr));
 	err = mlx5_glue->query_device_ex(ctx, NULL, &attr_ex);
-	if (err)
-		return err;
+	if (err) {
+		rte_errno = errno;
+		return -rte_errno;
+	}
 	device_attr->device_cap_flags_ex = attr_ex.device_cap_flags_ex;
 	device_attr->max_qp_wr = attr_ex.orig_attr.max_qp_wr;
 	device_attr->max_sge = attr_ex.orig_attr.max_sge;
@@ -170,8 +172,10 @@ mlx5_os_get_dev_attr(struct mlx5_common_device *cdev,
 
 	struct mlx5dv_context dv_attr = { .comp_mask = 0 };
 	err = mlx5_glue->dv_query_device(ctx, &dv_attr);
-	if (err)
-		return err;
+	if (err) {
+		rte_errno = errno;
+		return -rte_errno;
+	}
 
 	device_attr->flags = dv_attr.flags;
 	device_attr->comp_mask = dv_attr.comp_mask;
@@ -195,7 +199,7 @@ mlx5_os_get_dev_attr(struct mlx5_common_device *cdev,
 	strlcpy(device_attr->fw_ver, attr_ex.orig_attr.fw_ver,
 		sizeof(device_attr->fw_ver));
 
-	return err;
+	return 0;
 }
 
 /**
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index cde8d022cd..0e154b88ed 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -1172,12 +1172,11 @@ mlx5_alloc_shared_dev_ctx(const struct mlx5_dev_spawn_data *spawn,
 	MLX5_ASSERT(spawn->max_port);
 	sh = mlx5_malloc(MLX5_MEM_ZERO | MLX5_MEM_RTE,
 			 sizeof(struct mlx5_dev_ctx_shared) +
-			 spawn->max_port *
-			 sizeof(struct mlx5_dev_shared_port),
+			 spawn->max_port * sizeof(struct mlx5_dev_shared_port),
 			 RTE_CACHE_LINE_SIZE, SOCKET_ID_ANY);
 	if (!sh) {
-		DRV_LOG(ERR, "shared context allocation failure");
-		rte_errno  = ENOMEM;
+		DRV_LOG(ERR, "Shared context allocation failure.");
+		rte_errno = ENOMEM;
 		goto exit;
 	}
 	pthread_mutex_init(&sh->txpp.mutex, NULL);
@@ -1199,9 +1198,8 @@ mlx5_alloc_shared_dev_ctx(const struct mlx5_dev_spawn_data *spawn,
 	strncpy(sh->ibdev_path, mlx5_os_get_ctx_device_path(sh->cdev->ctx),
 		sizeof(sh->ibdev_path) - 1);
 	/*
-	 * Setting port_id to max unallowed value means
-	 * there is no interrupt subhandler installed for
-	 * the given port index i.
+	 * Setting port_id to max unallowed value means there is no interrupt
+	 * subhandler installed for the given port index i.
 	 */
 	for (i = 0; i < sh->max_port; i++) {
 		sh->port[i].ih_port_id = RTE_MAX_ETHPORTS;
@@ -1211,12 +1209,12 @@ mlx5_alloc_shared_dev_ctx(const struct mlx5_dev_spawn_data *spawn,
 		sh->td = mlx5_devx_cmd_create_td(sh->cdev->ctx);
 		if (!sh->td) {
 			DRV_LOG(ERR, "TD allocation failure");
-			err = ENOMEM;
+			rte_errno = ENOMEM;
 			goto error;
 		}
 		if (mlx5_setup_tis(sh)) {
 			DRV_LOG(ERR, "TIS allocation failure");
-			err = ENOMEM;
+			rte_errno = ENOMEM;
 			goto error;
 		}
 		err = mlx5_rxtx_uars_prepare(sh);
@@ -1246,19 +1244,19 @@ mlx5_alloc_shared_dev_ctx(const struct mlx5_dev_spawn_data *spawn,
 	pthread_mutex_unlock(&mlx5_dev_ctx_list_mutex);
 	return sh;
 error:
+	err = rte_errno;
 	pthread_mutex_destroy(&sh->txpp.mutex);
 	pthread_mutex_unlock(&mlx5_dev_ctx_list_mutex);
 	MLX5_ASSERT(sh);
-	if (sh->td)
-		claim_zero(mlx5_devx_cmd_destroy(sh->td));
+	mlx5_rxtx_uars_release(sh);
 	i = 0;
 	do {
 		if (sh->tis[i])
 			claim_zero(mlx5_devx_cmd_destroy(sh->tis[i]));
 	} while (++i < (uint32_t)sh->bond.n_port);
-	mlx5_rxtx_uars_release(sh);
+	if (sh->td)
+		claim_zero(mlx5_devx_cmd_destroy(sh->td));
 	mlx5_free(sh);
-	MLX5_ASSERT(err > 0);
 	rte_errno = err;
 	return NULL;
 }
diff --git a/drivers/net/mlx5/windows/mlx5_flow_os.c b/drivers/net/mlx5/windows/mlx5_flow_os.c
index 7bb4c4590a..f5e3893ed4 100644
--- a/drivers/net/mlx5/windows/mlx5_flow_os.c
+++ b/drivers/net/mlx5/windows/mlx5_flow_os.c
@@ -372,7 +372,7 @@ mlx5_flow_os_init_workspace_once(void)
 
 	if (err) {
 		DRV_LOG(ERR, "Can't create flow workspace data thread key.");
-		return err;
+		return -rte_errno;
 	}
 	pthread_mutex_init(&lock_thread_list, NULL);
 	return 0;
diff --git a/drivers/net/mlx5/windows/mlx5_os.c b/drivers/net/mlx5/windows/mlx5_os.c
index 3db33cd0cf..a9619e5bda 100644
--- a/drivers/net/mlx5/windows/mlx5_os.c
+++ b/drivers/net/mlx5/windows/mlx5_os.c
@@ -152,7 +152,7 @@ mlx5_init_once(void)
  *   Pointer to mlx5 device attributes.
  *
  * @return
- *   0 on success, non zero error number otherwise.
+ *   0 on success, a negative errno value otherwise and rte_errno is set.
  */
 int
 mlx5_os_get_dev_attr(struct mlx5_common_device *cdev,
@@ -161,10 +161,11 @@ mlx5_os_get_dev_attr(struct mlx5_common_device *cdev,
 	struct mlx5_context *mlx5_ctx;
 	void *pv_iseg = NULL;
 	u32 cb_iseg = 0;
-	int err = 0;
 
-	if (!cdev || !cdev->ctx)
-		return -EINVAL;
+	if (!cdev || !cdev->ctx) {
+		rte_errno = EINVAL;
+		return -rte_errno;
+	}
 	mlx5_ctx = (struct mlx5_context *)cdev->ctx;
 	memset(device_attr, 0, sizeof(*device_attr));
 	device_attr->max_cq = 1 << cdev->config.hca_attr.log_max_cq;
@@ -187,15 +188,14 @@ mlx5_os_get_dev_attr(struct mlx5_common_device *cdev,
 	pv_iseg = mlx5_glue->query_hca_iseg(mlx5_ctx, &cb_iseg);
 	if (pv_iseg == NULL) {
 		DRV_LOG(ERR, "Failed to get device hca_iseg");
-		return errno;
-	}
-	if (!err) {
-		snprintf(device_attr->fw_ver, 64, "%x.%x.%04x",
-			MLX5_GET(initial_seg, pv_iseg, fw_rev_major),
-			MLX5_GET(initial_seg, pv_iseg, fw_rev_minor),
-			MLX5_GET(initial_seg, pv_iseg, fw_rev_subminor));
+		rte_errno = errno;
+		return -rte_errno;
 	}
-	return err;
+	snprintf(device_attr->fw_ver, 64, "%x.%x.%04x",
+		 MLX5_GET(initial_seg, pv_iseg, fw_rev_major),
+		 MLX5_GET(initial_seg, pv_iseg, fw_rev_minor),
+		 MLX5_GET(initial_seg, pv_iseg, fw_rev_subminor));
+	return 0;
 }
 
 /**
-- 
2.25.1